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

Lots of fixes from various forks #20

Closed martinb3 closed 9 years ago

martinb3 commented 10 years ago
martinb3 commented 10 years ago

@patcon This now accepts the original event list, as well as a filter syntax like issues:comment,issues:close,pull_request,push:*. I've tested it on my hubot instance and it works great.

I think this addresses your concern, and also merges in a bunch of improvements from you, @mattrudder, and @pmgarman.

patcon commented 10 years ago

This looks rad, @martinb3 :)

I haven't been using this script in awhile, but I'll spin up a bot and test it out tomorrow. Thanks again!

patcon commented 10 years ago

ping @whit537 -- if it's easier, you could just transfer the repo to me, and I'll transfer it to the hubot-scripts org

patcon commented 10 years ago

@martinb3 Ah, I'm just realizing that this is an omni-merge of lots of unrelated changes from a few people, which makes it a bit difficult to discuss and review.

I'll try to work through this later, but while it might seem easier, I might suggest that many maintainers wouldn't agree this is good form. I very much appreciate the time it took, but separate pull requests for atomic issues is much preferable in the future, especially since we don't have tests :)

martinb3 commented 10 years ago

@patcon I'm 100% with you on that, but it seems like no one is submitting much back upstream. These are mostly not my changes, and it seems silly for me to go submit them individually, especially when I've already done the work to integration test them. Anything I've done originally, I would definitely submit as individual feature branches with PRs :)

patcon commented 10 years ago

Oh hey, no worries about splitting it up yourself. I've just got some yak-shaving to do to figure out how testing works for notifier scripts, so I might be slow :)

n0mer commented 9 years ago

@martinb3 , @patcon any progress with testing?

1st year anniversary for this PR is coming :)

martinb3 commented 9 years ago

I don't think any testing was outstanding on my part, but I'm not using this plugin anymore, @n0mer, but this PR was just an amalgam of different forks that had various fixes.

martinb3 commented 9 years ago

I'll close this but leave the branch in case someone wants to reopen/merge; but since I'm not using it anymore, it seems silly to have the request come from me.