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

Add configuration option for commit msg format #52

Closed
Krinkle opened this issue Apr 20, 2012 · 11 comments
Closed

Add configuration option for commit msg format #52

Krinkle opened this issue Apr 20, 2012 · 11 comments

Comments

@Krinkle
Copy link
Contributor

Krinkle commented Apr 20, 2012

Right now it is hardcoded on line 221:

                msg = "Close GH-" + id + ": " + pull.title + ".",

jQuery projects usually prefer the format of "Landed pull request #{id}: {title}". Maybe add a config option for this with a a $1 and $2 as placeholder for id and pull.title.

@mikesherov
Copy link
Contributor

The message must include "close GH-" to auto close the PR. But other than that, cool!

@Krinkle
Copy link
Contributor Author

Krinkle commented Apr 20, 2012

Isn't there an API method to close it? Since jQuery landings usually don't contain that. Neither does GitHub's own "Merge" button use that (although GitHub's own button doesn't mean anything since it could just close the pr internally)

@mikesherov
Copy link
Contributor

Using the github auto close does a bunch of nifty things that the API alone does not, like automated linking of commit to PR. Otherwise, we have to make 2 API calls to simulate (but not exactly match) the same thing, which wastes time and causes a bunch of unneeded code. How about "landing pull request closes GH-###"?

@Krinkle
Copy link
Contributor Author

Krinkle commented Apr 20, 2012

Sure, anything is possible. The user can configure whatever format he/she wants, this is merely requesting the ability to modify it from the config. The default can stay the same if you want.

@staabm
Copy link

staabm commented Apr 20, 2012

Github does also close PR or Issues when using "closes #XXX" or "closed #XXX" inside the commit message. The "GH-" prefix is not necessary.

@mikesherov
Copy link
Contributor

@staabm, but if you're using an external tracker, "closes" confuses the external tracker. Please see #47 for more info.

@mikesherov
Copy link
Contributor

@Krinkle, the msg MUST at least contain "Close GH-" + id. I'll look into having additional formatting beyond that. Does that suffice?

@Krinkle
Copy link
Contributor Author

Krinkle commented Apr 20, 2012

I think @sindresorhus made a good point in #47 (although he later agreed to change it to GH-{id}). For repositories just using plain github, the "GH-" prefix looks foreign and redundant. They're already on github, there is no need to prefix anything with "Github". For those projects it make sense to to easily override the message to say "Twirling pull request #{id} onto the repository dance flour.", especially since on GitHub issues and pull requests share IDs and workflow, there is no need for distinction.

Again, what the default is or what projects with external issue trackers use (like jQuery core), is something else.

@sindresorhus
Copy link
Contributor

Mike made a good point about it fudding with a lot of external bug trackers. If we added a template, I still stand by initial thought of catering primarily for users of the GitHub issue tracker, with others having the option to override.

What about giving the user the pull request id, title and bugs and let them do whatever they want with it?

This being the default:
pulley.template = 'Close #{id}: {title} - {bugs}'

Which would output:

Close #123: Awesome pull request - Fixes #321

jQuery could override with this template:

pulley.template = 'Close GH-{id}: {title} - {bugs}'

Thoughts?

@Krinkle
Copy link
Contributor Author

Krinkle commented Apr 21, 2012

@sindresorhus Just now on IRC @mikesherov and I were basically coming to the same conclusion. See also #23 and this StackOverflow thread. We'll have to add a little bit of detection to see if the custom message is still going to make github auto-close it, and if not, do an Api call to close it afterwards.

@sindresorhus
Copy link
Contributor

Can use some of the regex from @mikesherov in #31:

findBug = /(?:resolved|close|closes|closed|fix|fixes|fixed)\s+#(\d+)/ig,

@Krinkle Krinkle closed this as completed Dec 18, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants