A reminder about commit messages: they should be useful

2017-04-17 Thread Boris Zbarsky
A quick reminder to patch authors and reviewers. Changesets should have commit messages. The commit message should describe not just the "what" of the change but also the "why". This is especially true in cases when the "what" is obvious from the diff anyway; for larger changes it makes sens

Re: A reminder about commit messages: they should be useful

2017-04-17 Thread David Major
I'd like to add to this a reminder that commit messages should describe the _change_ and not the _symptom_. In other words, "Bug XYZ: Crash at Foo::Bar" is not a good summary. This is implied by what Boris said, but I've seen enough of these on my pulsebot backscroll that it's worth mentioning exp

Re: A reminder about commit messages: they should be useful

2017-04-17 Thread James Graham
On 17/04/17 16:41, David Major wrote: I'd like to add to this a reminder that commit messages should describe the _change_ and not the _symptom_. In other words, "Bug XYZ: Crash at Foo::Bar" is not a good summary. An unfortunate pattern I see is non-descriptive commit messages for tests, which

Re: A reminder about commit messages: they should be useful

2017-04-17 Thread Gregory Szorc
On Mon, Apr 17, 2017 at 8:16 AM, Boris Zbarsky wrote: > A quick reminder to patch authors and reviewers. > > Changesets should have commit messages. The commit message should > describe not just the "what" of the change but also the "why". This is > especially true in cases when the "what" is o

Re: A reminder about commit messages: they should be useful

2017-04-17 Thread smaug
On 04/17/2017 06:16 PM, Boris Zbarsky wrote: A quick reminder to patch authors and reviewers. Changesets should have commit messages. The commit message should describe not just the "what" of the change but also the "why". This is especially true in cases when the "what" is obvious from the d

Re: A reminder about commit messages: they should be useful

2017-04-17 Thread Gregory Szorc
On Mon, Apr 17, 2017 at 4:10 PM, smaug wrote: > On 04/17/2017 06:16 PM, Boris Zbarsky wrote: > >> A quick reminder to patch authors and reviewers. >> >> Changesets should have commit messages. The commit message should >> describe not just the "what" of the change but also the "why". This is >>

Re: A reminder about commit messages: they should be useful

2017-04-17 Thread smaug
On 04/18/2017 02:36 AM, Gregory Szorc wrote: On Mon, Apr 17, 2017 at 4:10 PM, smaug wrote: On 04/17/2017 06:16 PM, Boris Zbarsky wrote: A quick reminder to patch authors and reviewers. Changesets should have commit messages. The commit message should describe not just the "what" of the cha

Re: A reminder about commit messages: they should be useful

2017-04-17 Thread gsquelart
On Tuesday, April 18, 2017 at 11:58:11 AM UTC+12, smaug wrote: > On 04/18/2017 02:36 AM, Gregory Szorc wrote: > > On Mon, Apr 17, 2017 at 4:10 PM, smaug wrote: > > > >> On 04/17/2017 06:16 PM, Boris Zbarsky wrote: > >> > >>> A quick reminder to patch authors and reviewers. > >>> > >>> Changesets s

Re: A reminder about commit messages: they should be useful

2017-04-17 Thread Gregory Szorc
On Mon, Apr 17, 2017 at 5:12 PM, wrote: > On Tuesday, April 18, 2017 at 11:58:11 AM UTC+12, smaug wrote: > > On 04/18/2017 02:36 AM, Gregory Szorc wrote: > > > On Mon, Apr 17, 2017 at 4:10 PM, smaug wrote: > > > > > >> On 04/17/2017 06:16 PM, Boris Zbarsky wrote: > > >> > > >>> A quick reminder

Re: A reminder about commit messages: they should be useful

2017-04-17 Thread smaug
On 04/18/2017 03:12 AM, gsquel...@mozilla.com wrote: On Tuesday, April 18, 2017 at 11:58:11 AM UTC+12, smaug wrote: On 04/18/2017 02:36 AM, Gregory Szorc wrote: On Mon, Apr 17, 2017 at 4:10 PM, smaug wrote: On 04/17/2017 06:16 PM, Boris Zbarsky wrote: A quick reminder to patch authors and

Re: A reminder about commit messages: they should be useful

2017-04-17 Thread Nicholas Nethercote
On Tue, Apr 18, 2017 at 9:58 AM, smaug wrote: > > That is why we have links to the bug. Bug should always be the unite of > truth telling > why some change was done. Bugs tend to have so much more context about the > change than any individual commit message can or should have. > With all due re

Re: A reminder about commit messages: they should be useful

2017-04-17 Thread Ben Kelly
On Mon, Apr 17, 2017 at 9:21 PM, Nicholas Nethercote wrote: > > That is why we have links to the bug. Bug should always be the unite of > > truth telling > > why some change was done. Bugs tend to have so much more context about > the > > change than any individual commit message can or should ha

Re: A reminder about commit messages: they should be useful

2017-04-17 Thread Jim Blandy
It seems like there is actually not a consensus on this. (I had thought Smaug's view was the consensus, and found bz's post surprising.) Both approaches have tradeoffs. There's a good reason we require the bug number front and center in a commit message. But I dare you to read the Web Replay bug <

Re: A reminder about commit messages: they should be useful

2017-04-17 Thread Nicholas Nethercote
On Tue, Apr 18, 2017 at 11:34 AM, Ben Kelly wrote: > FWIW I agree with Olli. I look for a good one line summary of the change, > but beyond that I find you really do need to look at the bug to get the > full context. > Huh, interesting. Thanks for the data point. > I don't object to people wr

Re: A reminder about commit messages: they should be useful

2017-04-17 Thread Boris Zbarsky
On 4/17/17 10:45 PM, Jim Blandy wrote: It seems like there is actually not a consensus on this. (I had thought Smaug's view was the consensus, and found bz's post surprising.) Really? I know where Olli is coming from, but even in his view a commit message like the one I was talking about is n

Re: A reminder about commit messages: they should be useful

2017-04-17 Thread L. David Baron
On Monday 2017-04-17 23:20 -0400, Boris Zbarsky wrote: > On 4/17/17 10:45 PM, Jim Blandy wrote: > > It seems like there is actually not a consensus on this. (I had thought > > Smaug's view was the consensus, and found bz's post surprising.) > > Really? I know where Olli is coming from, but even i

Re: A reminder about commit messages: they should be useful

2017-04-17 Thread Steve Fink
On 04/17/2017 08:11 PM, Nicholas Nethercote wrote: On Tue, Apr 18, 2017 at 11:34 AM, Ben Kelly wrote: I don't object to people writing longer commit messages, but that information needs to be in the bug. Our tools today (splinter and mozreview) don't do that automatically AFAIK. I think ther

Re: A reminder about commit messages: they should be useful

2017-04-17 Thread Gregory Szorc
On Mon, Apr 17, 2017 at 10:36 PM, Steve Fink wrote: > On 04/17/2017 08:11 PM, Nicholas Nethercote wrote: > >> On Tue, Apr 18, 2017 at 11:34 AM, Ben Kelly wrote: >> >> I don't object to people writing longer commit messages, but that >>> information needs to be in the bug. Our tools today (splin

Re: A reminder about commit messages: they should be useful

2017-04-18 Thread Anne van Kesteren
On Mon, Apr 17, 2017 at 5:16 PM, Boris Zbarsky wrote: > A quick reminder to patch authors and reviewers. Is this documented somewhere so you can just point folks to documentation if they get it wrong? E.g., for WHATWG standards there is a README.md that (sometimes indirectly) points to https://gi

Re: A reminder about commit messages: they should be useful

2017-04-18 Thread Mike Hommey
On Tue, Apr 18, 2017 at 02:10:21AM +0300, smaug wrote: > On 04/17/2017 06:16 PM, Boris Zbarsky wrote: > > A quick reminder to patch authors and reviewers. > > > > Changesets should have commit messages. The commit message should describe > > not just the "what" of the change but also the "why".

Re: A reminder about commit messages: they should be useful

2017-04-18 Thread Mike Hommey
On Tue, Apr 18, 2017 at 02:58:05AM +0300, smaug wrote: > On 04/18/2017 02:36 AM, Gregory Szorc wrote: > > On Mon, Apr 17, 2017 at 4:10 PM, smaug wrote: > > > > > On 04/17/2017 06:16 PM, Boris Zbarsky wrote: > > > > > > > A quick reminder to patch authors and reviewers. > > > > > > > > Changeset

Re: A reminder about commit messages: they should be useful

2017-04-18 Thread Mike Hommey
On Mon, Apr 17, 2017 at 09:34:39PM -0400, Ben Kelly wrote: > On Mon, Apr 17, 2017 at 9:21 PM, Nicholas Nethercote > wrote: > > > > That is why we have links to the bug. Bug should always be the unite of > > > truth telling > > > why some change was done. Bugs tend to have so much more context abo

Re: A reminder about commit messages: they should be useful

2017-04-18 Thread Mike Hommey
On Tue, Apr 18, 2017 at 03:42:40AM +0300, smaug wrote: > On 04/18/2017 03:12 AM, gsquel...@mozilla.com wrote: > > On Tuesday, April 18, 2017 at 11:58:11 AM UTC+12, smaug wrote: > > > On 04/18/2017 02:36 AM, Gregory Szorc wrote: > > > > On Mon, Apr 17, 2017 at 4:10 PM, smaug wrote: > > > > > > > >

Re: A reminder about commit messages: they should be useful

2017-04-18 Thread Ehsan Akhgari
On 2017-04-18 12:30 AM, Mike Hommey wrote: >> I've yet to see that to happen. What is crucial is fast way to browse >> through the blame in time. So commit messages should be short and >> descriptive. Telling what and why. (I very often need to go back to CVS era >> code). I won't spend time readin

Re: A reminder about commit messages: they should be useful

2017-04-18 Thread Boris Zbarsky
On 4/18/17 3:00 AM, Anne van Kesteren wrote: On Mon, Apr 17, 2017 at 5:16 PM, Boris Zbarsky wrote: A quick reminder to patch authors and reviewers. Is this documented somewhere so you can just point folks to documentation if they get it wrong? Not yet. As this thread shows, there's some li

Re: A reminder about commit messages: they should be useful

2017-04-18 Thread Daniel Holbert
The very basics (at least) are semi-documented here: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Committing_Rules_and_Responsibilities#Checkin_comment I frequently point people at that ^^ when they get it wrong (and e.g. use the bug title as the commit message). ~Daniel On

Re: A reminder about commit messages: they should be useful

2017-04-18 Thread smaug
On 04/18/2017 04:24 PM, Ehsan Akhgari wrote: On 2017-04-18 12:30 AM, Mike Hommey wrote: I've yet to see that to happen. What is crucial is fast way to browse through the blame in time. So commit messages should be short and descriptive. Telling what and why. (I very often need to go back to CVS

Re: A reminder about commit messages: they should be useful

2017-04-25 Thread Alexander Surkov
On Tuesday, April 18, 2017 at 6:53:14 PM UTC-4, smaug wrote: > On 04/18/2017 04:24 PM, Ehsan Akhgari wrote: > > On 2017-04-18 12:30 AM, Mike Hommey wrote: > >>> I've yet to see that to happen. What is crucial is fast way to browse > >>> through the blame in time. So commit messages should be short

Re: A reminder about commit messages: they should be useful

2017-04-25 Thread Boris Zbarsky
On 4/25/17 10:50 AM, Alexander Surkov wrote: I don't want to affirm that this approach suites every Mozilla module, but it seems be working well in relatively small modules like accessibility one. Just as a counterpoint... as non-regular contributor to the accessibility module, I have a _very

Re: A reminder about commit messages: they should be useful

2017-04-25 Thread Alexander Surkov
On Tuesday, April 25, 2017 at 11:11:28 AM UTC-4, Boris Zbarsky wrote: > On 4/25/17 10:50 AM, Alexander Surkov wrote: > > I don't want to affirm that this approach suites every Mozilla module, but > > it seems be working well in relatively small modules like accessibility one. > > Just as a counte

Re: A reminder about commit messages: they should be useful

2017-04-25 Thread Boris Zbarsky
On 4/25/17 1:07 PM, Alexander Surkov wrote: I bet there's always room for improvements, and I hope this was a counterpoint for the example only, not for the bug organization approach. Sort of. It was a counterpoint to "just check the bug; all the info is there". Often it's not there, or not

Re: A reminder about commit messages: they should be useful

2017-04-25 Thread smaug
On 04/25/2017 08:20 PM, Boris Zbarsky wrote: On 4/25/17 1:07 PM, Alexander Surkov wrote: I bet there's always room for improvements, and I hope this was a counterpoint for the example only, not for the bug organization approach. Sort of. It was a counterpoint to "just check the bug; all the

Re: A reminder about commit messages: they should be useful

2017-04-25 Thread Alexander Surkov
On Tuesday, April 25, 2017 at 1:20:29 PM UTC-4, Boris Zbarsky wrote: > On 4/25/17 1:07 PM, Alexander Surkov wrote: > > I bet there's always room for improvements, and I hope this was a > > counterpoint for the example only, not for the bug organization approach. > > Sort of. > > It was a counter

Re: A reminder about commit messages: they should be useful

2017-04-26 Thread Boris Zbarsky
On 4/25/17 4:27 PM, Alexander Surkov wrote: Maybe we should have a style guide, explaining what makes a good commit message and what makes a good and descriptive bug, with number of (good and bad) examples. Yes, we should. Maybe we should have a discussion at the all hands about this... -Bo

Re: A reminder about commit messages: they should be useful

2017-04-26 Thread Selena Deckelmann
Hi! On Wed, Apr 26, 2017 at 7:05 AM Boris Zbarsky wrote: > On 4/25/17 4:27 PM, Alexander Surkov wrote: > > Maybe we should have a style guide, explaining what makes a good commit > message and what makes a good and descriptive bug, with number of (good and > bad) examples. > > Yes, we should. >