Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow templating of notification msgs #18

Open
patcon opened this issue May 14, 2014 · 3 comments
Open

Allow templating of notification msgs #18

patcon opened this issue May 14, 2014 · 3 comments

Comments

@patcon
Copy link
Member

patcon commented May 14, 2014

#17 ideally shouldn't have needed to be a PR

Would be nice to do something with mustache like we do here:
https://github.com/gittip/hubot-heroku-deploy-notifier/blob/master/src/heroku-deploy-notifier.coffee

Since we have lots of diff event types, could just check an envvar derived from eventType (ie. HUBOT_GITHUB_EVENT_NOTIFIER_TEMPLATE_PULL_REQUEST)

Related: gratipay/roobot#55

@parkr
Copy link
Member

parkr commented May 14, 2014

I'd be 👇 like a 🐶 (in a good way) for templates, definitely an awesome idea. What templating engine would you use? I can't think of anything built-in to CoffeeScript. I wrote this a few years ago:

//
// Fills a string containing special templating syntax with the data provided.
//
// Ex:
//    tmpl = "${name} got a ${grade} in ${course}.";
//    data = { name: "John", grade: "B", course: "Plant Pathology" };
//    tmpl.template(data); // outputs "John got a B in Plant Pathology."
//
String.prototype.template = function(data){
    var regex = null, completed = this.toString();
    for(el in data){
        regex = new RegExp('\\${'+ el +'}', 'g');
        completed = completed.replace(regex, data[el]);
    }
    return completed.toString();
};

@patcon
Copy link
Member Author

patcon commented May 14, 2014

Rockin. That looks super-elegant, but to be honest, I don't know enough about writing templating engines to know what goes into the full packages and why they seem to be prefered. I'd personally air on the side using something like mustache.js or hogan.js -- do you feel strongly that they're bloated?

@parkr
Copy link
Member

parkr commented May 15, 2014

I also don't know enough about writng templating engines to know what goes into the full packages. I think the added weight is features. If we're just talking about outputting variables, I don't think we need an entire package. If we think we'd want some of the features of mustache, then I'd say go for it. Hogan looks way too bloated for our use-case here. Your call.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants