When to do a review for a downported change?

[Andrew Haley]

On 5/14/19 4:02 PM, Aleksey Shipilev wrote:
> On 5/14/19 4:56 PM, Lindenmaier, Goetz wrote:
>> Do we really need reviews if a backport does not apply
>> for trivial reasons?  I always do them, but it seems quite some
>> overhead to me.
>> I consider at least changes in the context of a hunk and
>> changes in the copyright year as trivial.
> 
> I would say trivial conflict resolutions in the comments (not
> affecting the semantics of the code/docs) can be done without
> additional review. Copyright year adjustments are the examples of
> this. Just say that in Fix Request.
> 
> However, the changes that massage the backport to fit older APIs
> need to be reviewed for sanity. The internal APIs are not overly
> consistent at times, and the thing that you might think is a trivial
> substitution might just not be so. Another pair of eyes to look at
> that addon is good to have.

+1

> 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?

-- 
Andrew Haley
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671