Re: [dev-servo] Fwd: Re: Suggested code review workflow

[David Rajchenbach-Teller]

In Gecko, I have taken the habit of requesting long messages when the
patch is not self-contained. Plus, I am currently working on bugs that
would have been much easier to puzzle out if we (well, if I) had
explained in the long message why some changes were made.

In other words, +1 for requesting long messages.

Cheers,
 David

On 23/02/16 02:09, smaug wrote:
> Well, even long commit messages tend to not capture all the key pieces
> why certain change was made, and what it is supposed to do, and
> often it is the comments from the reviewer which are valuable, and links
> to other related bugs, dependency tracking etc.
> But I know this is all very subjective. Some people like commit messages
> and some prefer to read the bug.
> Perhaps I had different opinion if Gecko had always used more
> descriptive commit messages.
> 
> 
> -Olli
> ___
> dev-servo mailing list
> dev-servo@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-servo
___
dev-servo mailing list
dev-servo@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-servo

Re: [dev-servo] Fwd: Re: Suggested code review workflow

[smaug]

On 02/21/2016 02:02 AM, Boris Zbarsky wrote:

On 2/20/16 5:10 PM, Josh Matthews wrote:

(as a random comment, I never read multiline comments for Gecko. Only the
first line + the bug number. It is the bug where the relevant information
needs to be available. Whether it it available also elsewhere is less
important, IMHO.)


Having the info in a bug is nice, but having to read 30, or 150, or 500 bug 
comments to find it can be less nice...

I think Gecko tends to rely to much on the "it's in the bug" crutch and not put 
enough info into the commit message, personally.

-Boris


Well, even long commit messages tend to not capture all the key pieces why 
certain change was made, and what it is supposed to do, and
often it is the comments from the reviewer which are valuable, and links to 
other related bugs, dependency tracking etc.
But I know this is all very subjective. Some people like commit messages and 
some prefer to read the bug.
Perhaps I had different opinion if Gecko had always used more descriptive 
commit messages.


-Olli
___
dev-servo mailing list
dev-servo@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-servo

Re: [dev-servo] Fwd: Re: Suggested code review workflow

[Manish Goregaokar]

One thing that I've found pretty useful (at least when digging into the
history
of some Rust feature) is that the merge commits in both Rust and Servo
contain
the full contents of the pull request text (not comments, just the PR
message).
However, there still are "fixes #123" type PR messages since nobody wants to
retype what the issue says.


-Manish Goregaokar

On Sun, Feb 21, 2016 at 5:58 AM, Lars Bergstrom 
wrote:

> This may also be less of a big deal here at Mozilla, where there's
> (presumably?) only been one bug database since 1998 and will only be
> one forever, but when I was at MS, I had to make accessibility fixes
> in some code that was a bit more than 20 years old, and "fixes
> B1#2003" is really tough to track down, when the bug database for B1
> has long been retired and everybody who worked on it is either retired
> or Bill Gates (he wasn't retired yet).
>
> At least in Servo, I could certainly see a world where we move off of
> GitHub Issues some day, and telling people, "oh, you need to go to
> larsberg's git mirror of the old issues repo and use his hacked-up web
> frontend to look for the bug, but remember to subtract 1024, because
> he couldn't build nice UI..." is bad.
>
> So, just based on old battle wounds, I guess I'm more in the camp
> putting justification for a piece of code near the code itself, with a
> bug link mainly if there's some lengthy historical baggage, a comment
> thread that captures an old architecture war, etc.
> - Lars
>
>
> On Sat, Feb 20, 2016 at 6:02 PM, Boris Zbarsky  wrote:
> > On 2/20/16 5:10 PM, Josh Matthews wrote:
> >>
> >> (as a random comment, I never read multiline comments for Gecko. Only
> the
> >> first line + the bug number. It is the bug where the relevant
> information
> >> needs to be available. Whether it it available also elsewhere is less
> >> important, IMHO.)
> >
> >
> > Having the info in a bug is nice, but having to read 30, or 150, or 500
> bug
> > comments to find it can be less nice...
> >
> > I think Gecko tends to rely to much on the "it's in the bug" crutch and
> not
> > put enough info into the commit message, personally.
> >
> > -Boris
> > ___
> > dev-servo mailing list
> > dev-servo@lists.mozilla.org
> > https://lists.mozilla.org/listinfo/dev-servo
> ___
> dev-servo mailing list
> dev-servo@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-servo
>
___
dev-servo mailing list
dev-servo@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-servo

Re: [dev-servo] Fwd: Re: Suggested code review workflow

[Lars Bergstrom]

This may also be less of a big deal here at Mozilla, where there's
(presumably?) only been one bug database since 1998 and will only be
one forever, but when I was at MS, I had to make accessibility fixes
in some code that was a bit more than 20 years old, and "fixes
B1#2003" is really tough to track down, when the bug database for B1
has long been retired and everybody who worked on it is either retired
or Bill Gates (he wasn't retired yet).

At least in Servo, I could certainly see a world where we move off of
GitHub Issues some day, and telling people, "oh, you need to go to
larsberg's git mirror of the old issues repo and use his hacked-up web
frontend to look for the bug, but remember to subtract 1024, because
he couldn't build nice UI..." is bad.

So, just based on old battle wounds, I guess I'm more in the camp
putting justification for a piece of code near the code itself, with a
bug link mainly if there's some lengthy historical baggage, a comment
thread that captures an old architecture war, etc.
- Lars


On Sat, Feb 20, 2016 at 6:02 PM, Boris Zbarsky  wrote:
> On 2/20/16 5:10 PM, Josh Matthews wrote:
>>
>> (as a random comment, I never read multiline comments for Gecko. Only the
>> first line + the bug number. It is the bug where the relevant information
>> needs to be available. Whether it it available also elsewhere is less
>> important, IMHO.)
>
>
> Having the info in a bug is nice, but having to read 30, or 150, or 500 bug
> comments to find it can be less nice...
>
> I think Gecko tends to rely to much on the "it's in the bug" crutch and not
> put enough info into the commit message, personally.
>
> -Boris
> ___
> dev-servo mailing list
> dev-servo@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-servo
___
dev-servo mailing list
dev-servo@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-servo