When to do a review for a downported change?

[Aleksey Shipilev]

On 5/14/19 5:52 PM, Andrew Haley wrote:
> On 5/14/19 4:02 PM, Aleksey Shipilev wrote:
>> Aside, I think it is a good style (though optional) to post the diff
>> between the upstream patch and the backport -- it seems low-overhead
>> when there is the mq patch on top. This would also make reviews much
>> easier, and probably fits the backporting workflow too. That is what
>> I do anyway.
> 
> Post in what format, though? A diff of a diff?

Depends. Whatever fits the workflow, and maybe with workflow adjustments:
 a) Sometimes just copy-paste *.rej and explain why those are not applicable.
 b) Sometimes just inline the "addon" patches that fix the original change: in my workflow with mq
there are two patches: the original "backport" change with rejects, and the follow up patch that
resolves those rejects, hg qdiff the second one and paste it.
 c) Sometimes just the copy-pasted affected block "before" / "after", showing the adjustment made;
 d) Sometimes just the diff of a diff is enough to highlight the changes. If that is not available,
I would do that during review for a non-trivial backport _anyway_.
 e) Sometimes just pointing out which files and methods have differences vs upstream version, so we
can eyeball them.

I mean, anything that _you_ as reviewer, who sees the backport for the first time, would like to see
in order to understand what was done, quickly and exactly. If backport differences are indeed
trivial, then any of the ways above would amount to a copy-pasted paragraph in RFR, and it would
breeze through the review, making the whole thing low-overhead.

(This is not to say people here are not doing that. Just describing "the common sense" out loud.)

-Aleksey