This started life over as a comment I posted at #1059966-20: Change recommended format for commit messages (no more '#1234 by abc') but I'm moving it here at eliza411's suggestion to hopefully gather more feedback on the proposal...
Sorry I didn't suggest this sooner, but I just came up with it today while I was happily doing Git commits on a plane. ;)
Instead of:
Issue #1234 by xxxHow about this?
[#1234] by xxxWe're already used to referring to issues like that (or we should be), and it's less duplication when scanning logs, and less wasted horizontal space for the all-important summary line.
I don't think it's necessarily too late to change this. We're still in a transition period trying out all sorts of approaches to all sorts of things, so the "best practices" are still being discovered. I'll probably just adopt this practice for myself, (along with trying to properly credit authorship for people who contribute patches, even if it does take a tiny bit of extra work on my part and it's not the documented recommendation). But this one seems like something we could all just get behind, and is even less work than the currently documented suggestion.
Thoughts? ;)
Thanks,
-Derek
Comments
No!
No! That would make release notes easy, but we would have to remove the descriptions to make them not be duplicated. With that, commit logs would be useless without an internet/d.o connection.
I go by Josh The Geek.
Drupal.org
GitHub
Yes!
I interpreted @dww as suggesting:
[#1234] by xxx: Fixed yyy zzz ....In fact, I been doing just
[#1234]: Fixed yyy zzz...'NAME <USERNAME@UID.no-reply.drupal.org>'bfroehle: What I was saying
bfroehle: What I was saying is that once you create release notes, you will have duplication. Example: If [#123]'s title is Blah, then you would see:
#123: Blah: Fixed Blah. Useless druplication.I go by Josh The Geek.
Drupal.org
GitHub
I meant duplication, but I
I meant duplication, but I think druplication should be a word, too. :)
I go by Josh The Geek.
Drupal.org
GitHub
Exactly
Yes, that's exactly what I'm suggesting. Thanks for clarifying...
Release notes
Repeating myself from the issue, but...
p.s. @Josh The Geek: Ask Druplicon on IRC about "druplication" ... I taught it that word over a year ago. ;)
I don't understand what this
I don't understand what this buys us.
"Issue #xxx by foo, bar: Description" is plain English and doesn't contain any Drupalisms.
Why force people to learn a funny syntax when they come to our project from other projects? Why force the documentation of what the funny syntax does as part of commit message documentation?
what it buys us
My original proposal explained what it buys us, but I guess I'll repeat it a 3rd time to be clear ;)
Furthermore:
a) We're not forcing people to learn anything. Nothing explodes if you don't follow our suggested best practice (which is good, because there'd be a lot of explosions if that were the case).
b) We're already documenting the suggested format for a commit message. I'm proposing we change the suggestion to remove a duplicate word that appears in front of every commit.
c) You're being selective about what you call a "Drupalism". ;) "#1234" is a Drupalism, you're just used to it, so it doesn't strike you as such. There's got to be some magic syntax that links a commit to an issue. We already have magic syntax for linking comments to issues. I'm proposing we reuse the same syntax for commits.
Just for clarity, if you are
Just for clarity, if you are referring to change the the current commit message format:
Issue #[issue number] by [comma-separated usernames]: [Short summary of the change].to:
[#[issue number]] by [comma-separated usernames]: [Short summary of the change].I am ok, but if you are proposing to remove the "[Short summary of the change]" I will say it will be a really bad idea since like already mentioned it would be harder to follow changes from a simple git clone.
I suppose you are referring what I have described here, but my first reaction was to scream about removing summary, so hoping to avoid people screaming I am writing this :-p
tip: using the full version can help us to understand it completely
Agreed
Yes, sorry I was abbreviating in my post. I absolutely am in favor of concise English summaries in the commit messages. I'm only talking about
s/Issue #1234/[#1234]/. Everything else about our documented best practice would remain the same."#1234" isn't (just) a
"#1234" isn't (just) a Drupalism, though; that same "relate this commit to this issue" syntax works in Unfuddle, works in CodeBase, works in Trac, and many other (all?) tracking systems I've used. We probably stole it from one of them originally. :)
"[#1234]" however, isn't anything I've seen anywhere other than the Drupal issue queue filter. And it's a bizarre syntax that people don't know about unless someone's showed them.
And I understand we're not forcing anyone to do anything. But we want to attract the kind of contributors who want to conform to best practices, for better interoperability with the rest of the community. "Saves 4 keystrokes/slightly easier scanning" isn't a big enough argument to warrant asking people to break their patterns used in other projects, at least to me. Barriers to contribution--;
I'd really have to second
I'd really have to second this, I also use Redmine with different bug tracking solutions quite a bit, and it's really nice to have commits linked at the issue (we have an issue where we are looking to add this capability to drupal.org!).
Further, most of those systems also support updating the issue status from the commit message, which I would largely support as well. For example: "Fixes #12345 by webchick: Issue statuses can be set in commit messages!" would actually set issue 12345 to fixed. I would love to see this functionality land on d.o, or at least in vc_project for those that want it. Is it possible to hammer out a standard for this now, rather than later?
Definitely see the point and
Definitely see the point and usefulness of commits automatically updating the issue status. Unfuddle actually supports even more commit message keywords than "fixed" - there are also keywords to relate issues, and each keyword also works in many variations; i.e., "fix", "fixes", "fixed", "solves", "resolves", etc. If no keyword is matched, then a commit is only associated with an issue. See http://unfuddle.com/docs/topics/powerful_commit_messages for details.
However, careful - simply marking an issue as fixed along with the commit message would often not be sufficient. In my experience, a friendly and carefully crafted follow-up is perceived a lot better by users and contributors. That's why we've started the issue status transitions handbook page http://drupal.org/node/467548 that's actively used by many maintainers in the meantime.
Even more often, I have to amend a commit and status change to fixed with a clarification, follow-up questions, todos, and whatnot. As an ironically related example, have a look at http://drupal.org/node/1061758#comment-4371406
Of course, you'll now tell me that I can still follow up manually after committing - but bear in mind that the usual workflow is to 1) open the issue 2) download, apply, and commit the patch and 3) follow up on the still open issue. Hence, unless you also change the issue status in the form accordingly, you'll reset the status from fixed to its previous state...
Daniel F. Kudwien
netzstrategen
Simply a hyphen, please
Like webchick, I never liked that special bracket syntax for issues and am heavily opposed to it, as it's indeed not only a Drupalism, but also hard to type if you happen to not have a English/US keyboard layout. Actually, I can see how typing the brackets is not a problem for Americans, but on my non-US keyboard layout, it involves a relatively complex keystroke.
However, coming from the corresponding issue for Dreditor to change the auto-generated commit message template in http://drupal.org/node/1061758, I'd like to propose to change the "Issue " prefix to a simple "- " (hyphen) prefix.
Basically for the same reasons that caused this discussion: Less clutter, fast to type, no repetition, etc.pp
Daniel F. Kudwien
netzstrategen
Any feedback?
Any feedback?
Daniel F. Kudwien
netzstrategen
+1 for staying with 'Issue
+1 for staying with 'Issue #xxx: ...'
Senior Drupal Developer for Lullabot | www.davereid.net | @davereid
But why "Issue " and not
But why "Issue " and not simply "- " ?
Daniel F. Kudwien
netzstrategen
Because it's already stuck in
Because it's already stuck in devs' heads? I personally will just stick to issue. BTW, dreditor wants - in it's commit messages.
I go by Josh The Geek.
Drupal.org
GitHub
Because prepending "Issue"
Because prepending "Issue" adds some semantic value to the self-contained commit message (even if it's implied, it makes explicit the fact that the following number refers to an issue). The hyphen adds no semantic value; if anything, it detracts from the self-contained nature of a commit message by suggesting that this commit is a bullet in some larger list. What list? All commits? If it needs to be formatted as a list, that can (and should) be done external to the commit message. The hyphen is semantically null, it's got no business being somewhere that human eyes read.
Plus, "- " would be a massive Drupalism.
What Sam Said
It's not semantic and you should be using
<ul>and<li>yourself (or via drush release-notes) and not be adding list display in your commit messages.Senior Drupal Developer for Lullabot | www.davereid.net | @davereid
ok, but what's wrong in just
ok, but what's wrong in just #12345 format?
without any variants like "Issue #12345 blah blah'
I prefer make comments like 'Fixes for #12345' and after all '#12345 fixed' when it's fixed.
Why add different brackets there or some different additional words?
We a going use some powerful preprocessor language for git comments? :)
Because if you start a line
Because if you start a line in the git commit message with # it is treated as a commit and ignored, and that's a big no no.
I go by Josh The Geek.
Drupal.org
GitHub