search

hubot-archive / hubot-github-repo-event-notifier

Notifies about any GitHub repo event available via webhook.
https://www.npmjs.org/package/hubot-github-repo-event-notifier
57 stars 42 forks source link

Make Github messages more readable #34

Closed florasaramago closed 8 years ago

florasaramago commented 8 years ago

We've been using this script a lot with Mattermost at the company I work for, and we love it. But since we have many repositories and a lot of daily activity, our code channel was getting a little messy, so I made some changes to the messages to make them more readable. I hope other people might like it as well.

parkr commented 8 years ago

This is wonderful, thank you! ✨

Have you run this with your production hubot perchance? We don't have any tests for this (heh) so if you could try it and let me know that it's working for you in all the changed cases, that would be wonderful. 😄

Also, is this syntax a Mattermost-specific syntax? If so, do you know if there's a way to make this platform-agnostic? Slack, Mattermost, and Campfire all have their own link formats.

florasaramago commented 8 years ago

I'm so glad you like it! 😄

Yes, I made these changes in our production hubot first, and we have been using it like this for about a week. So far we didn't have any problems.

About the syntax, that's a good point. I checked Slack's documentation and apparently they use the same link format as Mattermost: https://api.slack.com/docs/message-formatting#linking_to_urls

But I can't find any documentation on how Campfire or HipChat handle links (let alone all the other platforms), so I'm guessing they probably don't have this option. I think one possibility would be to have a separate class that would receive the data object and return the formatted message according to the current adapter. What do you think?

parkr commented 8 years ago

I think one possibility would be to have a separate class that would receive the data object and return the formatted message according to the current adapter. What do you think?

I think that sounds great! I believe robot.adapter provides the adapter name. If Mattermost and slack use the same formatting, then it should be a quick fix to create the new class or function that doesn't check the value of robot.adapter (but receives it) and returns a formatted URL based on the input. Then if folks using HipChat or Campfire upgrade, they could update the function to format properly for them and I can make a new release then. How does that sound? So just pulling the formatting out into a function or class then dealing with other adapters later.

florasaramago commented 8 years ago

Done! I'm not sure I did it in the best possible way, because I'm just starting to get into this node/coffee/hubot world (I'm originally a Rails developer), so let me know if there's a better way to accomplish this. :)

But in any case, it works! The links are still showing nicely in Mattermost and I tested it the other way around too (displaying only the URL when the adapter is Mattermost), and it also works. Now it's just a matter of adding "whens" to the switch and it should work for any adapter!

florasaramago commented 8 years ago

Done! I fixed the order of the parameters too, it was no trouble at all 😄

parkr commented 8 years ago

This is GREAT! ✨ 🎉 💖

Thank you so much for your contribution and for sticking with me through my reviews. I'll merge and release this now!

parkr commented 8 years ago

Looks like I can't release to NPM. For now, you can add this to your package.json:

"hubot-github-repo-event-notifier": "https://github.com/hubot-scripts/hubot-github-repo-event-notifier.git#v1.7.0"

I think that should work. :)

florasaramago commented 8 years ago

Awesome!!!!! 🎉 🎉 🎉

Thank you too, for taking the time to review my code! I had a lot of fun with this. I'm getting addicted to Hubot, so I hope to make more contributions soon!