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

2016-02-23 Thread 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

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

2016-02-23 Thread 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

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

2016-02-21 Thread 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

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

2016-02-20 Thread 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