On 4/16/14, 10:02 AM, Bobby Holley wrote:
On Wed, Apr 16, 2014 at 9:47 AM, Robert Kaiser wrote:
That's a good step. IMHO, often another good step is to mostly separate
every patch to its own bug (I know there are cases where it might not makes
sense, but often it does) so that individual chunk
On Wed, Apr 16, 2014 at 9:47 AM, Robert Kaiser wrote:
> That's a good step. IMHO, often another good step is to mostly separate
> every patch to its own bug (I know there are cases where it might not makes
> sense, but often it does) so that individual chunks can be accounted for
> separately in
Karl Tomlinson schrieb:
Joshua Cranmer 🐧. writes:
On 4/13/2014 4:42 PM, Robert O'Callahan wrote:
Honestly, I think we're already pretty close to most of those
recommendations, most of the time.
Some experienced Mozillians are breaking up their large changes
well, but some are not. And many
On 4/14/14, 10:31 AM, smaug wrote:
As a reviewer I usually want to see _also_ a patch which contains all
the changes.
Otherwise it can be very difficult to see the big picture.
But sure, having large patches split to smaller pieces may help.
btw, if you have opinions about code review tools, se
>On 04/14/2014 12:42 AM, Robert O'Callahan wrote:
>> On Sat, Apr 12, 2014 at 8:29 AM, Gregory Szorc wrote:
>> As a reviewer one of the first things I do when reviewing a big patch is to
>> see if I can suggest a reasonable way to split it into smaller patches.
>
>As a reviewer I usually want to se
On 4/14/14, 8:31 AM, Boris Zbarsky wrote:
On 4/14/14 5:13 AM, Aryeh Gregor wrote:
But doesn't Mercurial hide all but the first line by
default in the places you'd normally look for it (e.g., log)?
The normal place I'd look for the detailed message is something like
https://hg.mozilla.org/mozi
On 04/14/2014 12:42 AM, Robert O'Callahan wrote:
On Sat, Apr 12, 2014 at 8:29 AM, Gregory Szorc wrote:
I came across the following articles on source control and code review:
* https://secure.phabricator.com/book/phabflavor/article/
recommendations_on_revision_control/
* https://secure.phabri
On 4/14/14 5:13 AM, Aryeh Gregor wrote:
But doesn't Mercurial hide all but the first line by
default in the places you'd normally look for it (e.g., log)?
The normal place I'd look for the detailed message is something like
https://hg.mozilla.org/mozilla-central/rev/, which shows the
On Sun, Apr 13, 2014 at 10:01 PM, Karl Tomlinson wrote:
> Very often I've found that the intended approach changes during the
> life of a bug, and there is no clear summary in the bug of what
> eventually was done. It is then necessary to go back through
> multiple revisions of the patch and asso
On Mon, Apr 14, 2014 at 5:01 AM, Karl Tomlinson wrote:
> Very often I've found that the intended approach changes during the
> life of a bug, and there is no clear summary in the bug of what
> eventually was done. It is then necessary to go back through
> multiple revisions of the patch and assoc
Joshua Cranmer 🐧. writes:
> On 4/13/2014 4:42 PM, Robert O'Callahan wrote:
>> Honestly, I think we're already pretty close to most of those
>> recommendations, most of the time.
Some experienced Mozillians are breaking up their large changes
well, but some are not. And many less experienced cont
I also added a link at
https://developer.mozilla.org/en-US/docs/Developer_Guide/How_to_Submit_a_Patch#Creating_a_patch
Benoit Girard writes:
> I didn't know this existed. I filed bug 995763 to get this link added to
> the 'review requested' email to hopefully increase visibility.
>
>
> On Sat, A
On Fri, 11 Apr 2014 13:29:01 -0700, Gregory Szorc wrote:
> https://secure.phabricator.com/book/phabflavor/article/writing_reviewable_code
> I would be thrilled if we started adopting some of the
> recommendations such as more descriptive commit messages and many,
> smaller commits over fewer, com
On 4/13/2014 4:42 PM, Robert O'Callahan wrote:
Honestly, I think we're already pretty close to most of those
recommendations, most of the time. "More descriptive commit messages"
is the only recommendation there that is not commonly followed, as far
as I can see.
I was thinking about it, and
On Sat, Apr 12, 2014 at 8:29 AM, Gregory Szorc wrote:
> I came across the following articles on source control and code review:
>
> * https://secure.phabricator.com/book/phabflavor/article/
> recommendations_on_revision_control/
> * https://secure.phabricator.com/book/phabflavor/article/
> writin
I didn't know this existed. I filed bug 995763 to get this link added to
the 'review requested' email to hopefully increase visibility.
On Sat, Apr 12, 2014 at 12:10 PM, Kartikaya Gupta wrote:
> Just a reminder that this page exists:
>
> https://developer.mozilla.org/en-US/docs/Developer_Guide/
Just a reminder that this page exists:
https://developer.mozilla.org/en-US/docs/Developer_Guide/Reviewer_Checklist
and you should feel free to add things to it, and use it when reviewing
any code (your own or other people's).
kats
On 11/4/2014, 17:47, Mike Conley wrote:
Whoa, didn't expect
Whoa, didn't expect to see a blog post I wrote in grad school to get
called out here. :) Interesting to see it show up on the radar.
Re-reading it, I think the most interesting thing about the Cohen study
that I absorbed was the value of reviewing my own code before requesting
review from other pe
Code review tool company SmartBear published an interesting study [1] of
the effectiveness of code reviews at Cisco. (They used SmartBear's
tools, of course.) Mozillian Mike Conley reviewed SmartBear's study on
his blog [2].
The results are interesting and actionable. Some highlights:
* Revie
19 matches
Mail list logo
