That seems mostly reasonable. I'd try to make it more concise, though. The coding standards and developer policy docs should be short. +Commit message +-------------- + +Although we don't enforce the format of commit messages, there are general +guidelines that will help review, search in logs, email formatting and so on. +Mostly, the rules that apply are similar to other git repositories, such as: Maybe just: "Commit messages should be formatted according to the following general guidelines:" +* Separate the commit message into title and body. + +* The title should not have more than 80 columns, although 60/70 would be + better, since that'd fit better into a `git log` or an email subject. + +* The body, if it exists, should be separated from the title by an empty line. + +* The body should be aligned to 80 columns and have as many paragraphs as + necessary, but not more than that. Meaning you should be concise, but + explanatory, including a complete reasoning, but leaving examples, code + snippets and gory details to bug comments, web review or the mailing list. I don't think the suggestion to be concise is worth putting in the document. More information is usually better. +* `Attribution of Changes`_ should be in a separate line, after the end of + the body, as simple as "Patch by John Doe.". + +* Text formatting and spelling should follow the same rules as documentation + and in-code comments, ex. capitalization, full stop, etc. This point seems borderline, but I guess some people need it. +While these rules match most of the cases, we're aware that some cases are not +covered, and that's why we don't think reverting patches with "bad" commit +messages is a reasonable thing to do. We can, however, remind people of this +section of the policy for future reference. I don't think this paragraph is needed, or it could be shortened to say that these are not hard and fast rules and developers should use their own editorial judgement. On Thu, Sep 25, 2014 at 5:47 AM, Renato Golin <renato.golin at linaro.org> wrote:> On 25 September 2014 00:56, Chandler Carruth <chandlerc at google.com> wrote: > > We could certainly have a little guide for how to write awesome commit > logs > > and point people at it. But this is one of (many) aspects of getting > ramped > > up on the practices and patterns of the community. Not sure it ever > really > > makes sense to try to codify so much as explain it... > > I agree we shouldn't codify or enforce, but educate and remind people > of good practices. > > How about the section attached? > > cheers, > --renato >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140925/7e9c1157/attachment.html>
On 25 September 2014 16:52, Reid Kleckner <rnk at google.com> wrote:> That seems mostly reasonable. I'd try to make it more concise, though. The > coding standards and developer policy docs should be short.Right, version 2 attached. Does everyone agree with it? cheers, --renato -------------- next part -------------- A non-text attachment was scrubbed... Name: commit-msg-policy.patch Type: text/x-patch Size: 1432 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140925/7a056a3c/attachment.bin>
----- Original Message -----> From: "Renato Golin" <renato.golin at linaro.org> > To: "Reid Kleckner" <rnk at google.com> > Cc: "Clang Dev" <cfe-dev at cs.uiuc.edu>, "LLVM Dev" <llvmdev at cs.uiuc.edu> > Sent: Thursday, September 25, 2014 11:14:37 AM > Subject: Re: [LLVMdev] [cfe-dev] Commit message policy? > > On 25 September 2014 16:52, Reid Kleckner <rnk at google.com> wrote: > > That seems mostly reasonable. I'd try to make it more concise, > > though. The > > coding standards and developer policy docs should be short. > > Right, version 2 attached. Does everyone agree with it?I think this looks good. I'd say, "because that fits better" instead of "since that'd fit better". Also, I'd like to add a note that all commit references should be to svn revision numbers, and not git hashes from a git mirror. In addition, all reverts must provide a rational. Thanks, Hal> > cheers, > --renato > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >-- Hal Finkel Assistant Computational Scientist Leadership Computing Facility Argonne National Laboratory
On Thu, Sep 25, 2014 at 9:14 AM, Renato Golin <renato.golin at linaro.org> wrote:> On 25 September 2014 16:52, Reid Kleckner <rnk at google.com> wrote: > > That seems mostly reasonable. I'd try to make it more concise, though. > The > > coding standards and developer policy docs should be short. > > Right, version 2 attached. Does everyone agree with it? >I don't specifically disagree with it, but I don't think all of it is useful. Let me be more specific though: +Commit message +-------------- + +Commit messages should be formatted according to the following general guidelines: If these are just suggestions (and it sounds like they are) I think you shouldn't call it policy and you shouldn't use the phrasing "should be formatted". If folks don't read to the end, they would get the wrong impression. I would probably phrase this as: """ Here are some guidelines for writing and formatting commit messages. The goal is to help your commit logs fit well into the various pieces of infrastructure and be useful to other LLVM developers both when reviewing your code and when investigating commits at some date in the future to understand why a change was made. """ + +* Separate the commit message into title and body. Useful. + +* The title should not have more than 80 columns, although 50~70 would be + better, since that'd fit better into a `git log` or an email subject. I don't think this is useful. Unless people are really diligent about trying to fit into 'git log' style displays, they won't actually abide by it. And if they are so diligent, they won't need the reminder. + +* The body, if it exists, should be separated from the title by an empty line. This part is totally useful (although it could be combined with the first bullet point). + It should be aligned to 80 columns and may contain multiple paragraphs. But I don't think telling people to do *this* will help -- either they wrap their text in their editor or they don't. If you want to do anything I would just make a single statement about column width along the lines of "as with the code in LLVM, commit logs are often best wrapped to 80 columns". + +* `Attribution of Changes`_ should be in a separate line, after the end of + the body, as simple as "Patch by John Doe.". Totally useful. + +* Text formatting and spelling should follow the same rules as documentation + and in-code comments, ex. capitalization, full stop, etc. Not really useful. The cost of formatting, punctuation, etc. mistakes in commit logs in near zero. We also can't fix them, so it doesn't seem to be terribly useful to spell out that fixes to these things are acceptable (the way it is useful for comments in code). My other high-level concern is that I feel like this misses the most important piece of guidance about commit logs: it doesn't provide suggestions about what *content* to put into the commit log! If we're going to give suggestions for developers about how to write their commit logs, it should include this. I'm willing to take a stab at drafting a version of this that includes such suggestions later today if that helps? -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140925/63c9608d/attachment.html>
Hi, I haven't got any comments to add that haven't already been discussed but I thought it would be worth mentioning that Phabricator's documentation has a fairly good section on good commit messages at https://secure.phabricator.com/book/phabflavor/article/writing_reviewable_code/#write-sensible-commit-me.> -----Original Message----- > From: cfe-dev-bounces at cs.uiuc.edu [mailto:cfe-dev-bounces at cs.uiuc.edu] > On Behalf Of Renato Golin > Sent: 25 September 2014 17:15 > To: Reid Kleckner > Cc: Clang Dev; LLVM Dev > Subject: Re: [cfe-dev] [LLVMdev] Commit message policy? > > On 25 September 2014 16:52, Reid Kleckner <rnk at google.com> wrote: > > That seems mostly reasonable. I'd try to make it more concise, though. The > > coding standards and developer policy docs should be short. > > Right, version 2 attached. Does everyone agree with it? > > cheers, > --renato