Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

Lars Schneider [off-list ref] writes:

    ... then I am always super
    eager to send out a new roll just because I don't want any other reviewer
    to waste time on obviously wrong patches. However, I have the impression
    that frequent re-rolls are frowned upon.

Correct.  A good middle-ground is to just reply with "Yes, thanks
for your suggestion, will fix in the next round", while receiving
review comments.  Good reviewers who value their time will not to
waste their time by responding on a point that has already been
pointed out and acknowledged.

4.) Reviewing patches is super hard for me because my email client does not
    support patch color highlighting and I can't easily expand context or look at
    the history of code touched by the patch (e.g via git blame). I tried to setup
    Alpine but I wasn't happy with the interface either. I like patches with a GitHub
    URL for review but then I need to find the right line in the original email to
    write a comment.

Unless a patch is about an area you are super familiar with so that
you know what is beyond the context of the patch to be able to judge
if the change is good in the context of the file being touched, it
is always hard to review from inside a mail reader.

Running "git am" is a good first step to review such a patch, as
that lets you view the resulting code with the full power of Git.
As you gain experience on the codebase, you'll be able to spot more
problems while in your mail reader.

Hi Junio,

On Mon, 8 Aug 2016, Junio C Hamano wrote:

Lars Schneider [off-list ref] writes:

quoted

4.) Reviewing patches is super hard for me because my email client
does not support patch color highlighting and I can't easily expand
context or look at the history of code touched by the patch (e.g via
git blame). I tried to setup Alpine but I wasn't happy with the
interface either. I like patches with a GitHub URL for review but then
I need to find the right line in the original email to write a
comment.

Unless a patch is about an area you are super familiar with so that you
know what is beyond the context of the patch to be able to judge if the
change is good in the context of the file being touched, it is always
hard to review from inside a mail reader.

Running "git am" is a good first step to review such a patch, as that
lets you view the resulting code with the full power of Git.  As you
gain experience on the codebase, you'll be able to spot more problems
while in your mail reader.

I am glad that you agree that the requirement to manually transform the
patches back into Git (where they had been originally to begin with) is
cumbersome. This is the first time that I see you admit it ;-)

Ciao,
Dscho