githubot
githubot copied to clipboard
iangreenleaf
Add commit formatting (optional oneline and gitio reduction)
I'd like to centralize commit formatting between Hubot scripts, and this seemed like an appropriate place. I've stubbed this out, but the tests are currently failing and I haven't had time to track down the reasons yet (assistance welcome!). Once this gets fixed up and released, we can use it in github/hubot-scripts#1461.
Ok, with 72a05c5 I'm passing local tests, so I think this is ready for review and merging.
The Travis errors appear to be due to incompatibility with older versions of Node. For example:
npm ERR! Error: No compatible version found: debug@'^0.8.1'
npm ERR! Valid install targets:
npm ERR! ["0.0.1","0.1.0","0.2.0","0.3.0","0.4.0","0.4.1","0.5.0","0.6.0","0.7.0","0.7.1","0.7.2","0.7.3","0.7.4","0.8.0","0.8.1","1.0.0","1.0.1"]
I didn't mess with any of that, so I expect the current master has the same failures independent of this pull request.
On Tue, Jun 10, 2014 at 11:46:12AM -0700, Parker Moore wrote:
Docs would also be nice for this.
Readme entries?
Readme entries?
Yurp. Adding the env vars, possible values, and what they do when set to each of those values.
Hmm, actually, I did add the gitio2 dependency. But the debug package is only being pulled in by nock (v0.32.3 requires debug ^0.8.1) and mocha (v1.20.0 required debug *).
On Tue, Jun 10, 2014 at 11:52:11AM -0700, Parker Moore wrote:
Adding the env vars, possible values, and what they do when set to each of those values.
Writing these up turned up a number of issues (e.g. commit.html_url is only set for one endpoint). But with 3288596 I think I've addressed all of that. Let me know if anything is unclear ;).
I'm undecided on whether I want to merge this or not. It's cool functionality and useful, but it perhaps strays a little bit from githubot's core mission.
At any rate, the test failures aren't your fault, something must have changed in the dependencies since the last time master ran on Travis. I re-ran it and master is erroring now too.
Ok, fixed those errors in master, so if you merge, this branch should go green too.
On Tue, Jun 10, 2014 at 04:12:00PM -0700, Ian Young wrote:
I'm undecided on whether I want to merge this or not. It's cool functionality and useful, but it perhaps strays a little bit from githubot's core mission.
Sure. I was looking for a lower-level place to put code shared between hubot-scripts' github-commits and github-commit-links (and possibly other scripts) to avoid copy-pasting the formatting code (as I'm doing in the current round of github/hubot-scripts#1461). githubot seemed like a good place to store code for processing GitHub API results, but this code could also live in a stand-alone github-commit-formatting package, or some such.
The formatting code is pretty small for a stand-alone package, so I'd rather include it in githubot. That makes life easier for consumers too (only one dependency), and for me (no need to recreate a bunch of packaging boilerplate). But if you feel that it's adding too much of a maintenance burden, I'll suck it up and create a new package ;).
On Tue, Jun 10, 2014 at 04:33:42PM -0700, Ian Young wrote:
Ok, fixed those errors in master, so if you merge, this branch should go green too.
Rebased and force-pushed.
On Tue, Jun 10, 2014 at 04:36:32PM -0700, W. Trevor King wrote:
Rebased and force-pushed.
Ping.
On Tue, Jun 10, 2014 at 04:36:32PM -0700, W. Trevor King wrote:
Rebased and force-pushed.
Ping again ;).
Thanks for the ping - this had definitely slipped a long ways down my inbox. I've come back around and now agree that this seems reasonable for inclusion into githubot. Since git.io is an official GitHub service, there's a certain logic to offering it as a built-in dependency here. So yeah, let's do it! Thanks for writing tests, that helps a ton.
One small suggestion: I'm thinking we could change the structure of these calls ever so slightly to make them stylistically consistent with the existing stuff (for ex, the deployments API), give it a bit more fluent of a feel, and separate the different bits of functionality. Here's what I'm thinking it could look like:
gh.commits "iangreenleaf/githubot", (commits) -> # The index action
gh.commits("iangreenleaf/githubot").details "01234567deadbeef", (commit) -> # Single commit
gh.commits.format commit, (result) -> # The standalone formatting - yes, I intended to omit the parens
Does that make sense?