On Mon, 2016-05-02 at 13:26 -0600, Simon Glass wrote: > Hi Scott, > > On 2 May 2016 at 13:03, Scott Wood <o...@buserror.net> wrote: > > On Mon, 2016-05-02 at 12:57 -0600, Simon Glass wrote: > > > Hi Scott, > > > > > > On 1 May 2016 at 17:34, Scott Wood <o...@buserror.net> wrote: > > > > On Sun, 2016-05-01 at 12:55 -0600, Simon Glass wrote: > > > > > Hi, > > > > > > > > > > On 30 April 2016 at 20:18, Vagrant Cascadian <vagr...@debian.org> > > > > > wrote: > > > > > > > > > > Please can you add a commit message? > > > > > > > > I don't understand these "empty/missing commit message" remarks when > > > > there's a > > > > one-line changelog (in the subject). Do you seriously want the same > > > > line > > > > repeated twice in the git commit, just so something shows up in the > > > > body > > > > of > > > > the e-mail? It's one thing if the commit warrants more than a single > > > > line > > > > (though it's still not accurate to say that the changelog is > > > > completely > > > > absent), but a spelling fix is about as trivial as it gets... > > > > -Scott > > > > > > > > > > It only takes a few seconds to add a commit message and I think it is > > > good practice. > > > > > > But if you want to allow commits with no message (other than > > > merge/release tag), then we should document it here: > > > http://www.denx.de/wiki/U-Boot/Patches > > > > There is a commit message. It is 'Fix spelling of "occurred"'. > > > > And that wiki link explicitly says, "Put a detailed description after the > > summary and blank line. If the summary line is sufficient to describe the > > change, you can omit the blank line and detailed description." > > OK I made a little update to make it more limited: > > "Put a detailed description after the summary and blank line. If the > summary line is sufficient to describe the change (e.g. it is a > trivial spelling correction or whitespace update), you can omit the > blank line and detailed description." > > Does that seem reasonable?
It's an example so it doesn't really limit anything -- if it did, I think that'd be way too specific. The criteria should be whether the single line adequately describes the patch (including justification, etc. if non-obvious). If it doesn't, then ask for a more detailed changelog, the same as you would if it contained more than one line but still didn't adequately describe the patch. > We should avoid submitting new drivers and forgetting a commit > message. I could see a new driver sometimes reasonably having a one-line commit message (which is not the same as "forgetting a commit message"). It's a driver for hardware <foo>. If there's nothing unusual to be noted about the driver, known limitations, etc. then what else is there to say? > Also some fixes are trivial (e.g. adding 1 to a result) but > the reason for them needs to be explained. Yes, but simply mandating text beyond the summary line is likely to get you stuff like: foo: Add 1 to result of blah() Add one to the result of blah(). Signed-off-by: whoever The problem there is not brevity, it's that important information is missing. > Sorry if I'm being picky on this. I've spent a *lot* of time over the > past few years digging into code and finding changes that are not > self-explanatory. That's why I am keen on people adding comments to > header files for API functions and struct members Yes, we could definitely use more API/struct documentation. I have no problem with pushing for better changelogs in cases where more information would be helpful. But we shouldn't mandate the changelog equivalent of: i++; /* add one to i */ > I think it helps for people to answer the question 'why should this > code be submitted?' when writing a commit message. For the record. in > this case, I would have added a message something like 'Occurred is > spelled incorrectly in a number of places. Fix these up to provide > consistency'. OK, but "fix spelling" followed by a diff showing the misspellings conveys the same information quite clearly. -Scott _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot