-
Notifications
You must be signed in to change notification settings - Fork 38
Add commit formatting (optional oneline and gitio reduction) #25
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
base: master
Are you sure you want to change the base?
Conversation
|
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: I didn't mess with any of that, so I expect the current master has the same failures independent of this pull request. |
|
Docs would also be nice for this. |
|
On Tue, Jun 10, 2014 at 11:46:12AM -0700, Parker Moore wrote:
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 |
|
On Tue, Jun 10, 2014 at 11:52:11AM -0700, Parker Moore wrote:
Writing these up turned up a number of issues (e.g. commit.html_url is |
|
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:
Sure. I was looking for a lower-level place to put code shared The formatting code is pretty small for a stand-alone package, so I'd |
Also add a few GitHub API wrappers, to mirror gh.branches. To distinguish between the API wrappers and the formatter, we use the presence of a callback in the initial call. So: gh.commits(repo, commit, cb) takes a repo name, commit hash, and callback; fetches a commit from GitHub; and calls the callback on the returned commit. On the other hand, gh.commits().format(commit, cb) takes a commit object (as returned from GitHub) and callback, and calls the callback on the formatted commit. The options formatting depends on two options, as discussed in the Readme: * gitio (set from HUBOT_GITIO) toggles git.io URL shortening * oneline (set from HUBOT_COMMIT_ONELINE) toggles message reduction to just the summary line. The bulk of the logic was ported from hubot-scripts [1,2], but I've adjusted the URL munging. When it's set [3], I leave commit.html_url alone. For the other endpoints [4,5], I try to parse the API URL and convert it in a single pass. I don't have access to an enterprise instance for testing [6], but I'm assuming the API URLs look like: :apiRoot/api/:apiVersion/repos/:owner/:repo/commits/:sha https://yourdomain.com/api/v3/repos/:owner/:repo/commits/:sha and the HTML URLs looks like: :apiRoot/repos/:owner/:repo/commit/:sha https://yourdomain.com/repos/:owner/:repo/commit/:sha [1]: https://github.com/github/hubot-scripts/blob/v2.5.14/src/scripts/github-commit-link.coffee [2]: https://github.com/github/hubot-scripts/blob/v2.5.14/src/scripts/github-commits.coffee [3]: https://developer.github.com/v3/repos/commits/#get-a-single-commit [4]: https://developer.github.com/v3/git/commits/#get-a-commit [5]: https://developer.github.com/v3/activity/events/types/#pushevent [6]: https://developer.github.com/v3/#schema
|
On Tue, Jun 10, 2014 at 04:33:42PM -0700, Ian Young wrote:
Rebased and force-pushed. |
|
On Tue, Jun 10, 2014 at 04:36:32PM -0700, W. Trevor King wrote:
Ping. |
|
On Tue, Jun 10, 2014 at 04:36:32PM -0700, W. Trevor King wrote:
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 parensDoes that make sense? |
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.