[11u] Sconpe of Review ... was RFR(S): 8241234: Unify monitor enter/exit runtime entries.

[Lindenmaier, Goetz]

Hi,

It is pointless to ask reviewers to judge the risk
because reviews are only done if the change had to be adapted.
There are complex changes that just apply clean and thus are
downported without review.  

Judging the risk is clearly a thing of the downporter. This is formulated 
in Rule 1 of Oracles's Updates 
description : http://openjdk.java.net/projects/jdk-updates/approval.html ,
and also mentioned in step 6 of jdk11us description: 
https://wiki.openjdk.java.net/display/JDKUpdates/How+to+contribute+a+fix
This is supposed to help the maintainer to decide about the risk.

The major task of the review is to make sure the downported change 
is correct, i.e. has the same effect on 11 as the original one.
Nevertheless, if there is a  reviewer and he feels bad about a 
change, he should communicate his concerns! 

Best regards,
  Goetz.


> -----Original Message-----
> From: Andrew Dinn <adinn at redhat.com>
> Sent: Wednesday, September 9, 2020 11:38 AM
> To: Andrew Haley <aph at redhat.com>; Lindenmaier, Goetz
> <goetz.lindenmaier at sap.com>; Doerr, Martin <martin.doerr at sap.com>;
> 'Severin Gehwolf' <sgehwolf at redhat.com>; 'hotspot-compiler-
> dev at openjdk.java.net' <hotspot-compiler-dev at openjdk.java.net>; jdk-
> updates-dev at openjdk.java.net
> Cc: Langer, Christoph <christoph.langer at sap.com>
> Subject: Re: [11u] RFR(S): 8241234: Unify monitor enter/exit runtime entries.
> 
> On 09/09/2020 09:23, Andrew Haley wrote:
> > Thinking about this some more: it really should be a reviewer's job to
> > object if a patch is likely to fail a risk-vs-reward test. Every patch
> > should be considered carefully in this way. It's hard for
> > inexperienced contributors to be able to make such judgements, so they
> > should ask on the list if it isn't clear.
> 
> I agree. Indeed, I have already been doing that for some of the more
> complex patches.
> 
> Another couple of things that should really be agreed between the
> backporter and a reviewer when a patch is complex or has wider-reaching
> side-effects than jst fixing the bug itself is whether 1) to limit the
> backport to some subset of the upstream change or even 2) to come up
> with a custom change that fixes the problem in a different way. That may
> involve the backporter asking for an early review of a preliminary
> patch. There's /nothing wrong/ with doing that.
> 
> > But we can say this much: crashes and Java language specification
> > failures will always qualify for fixes; performance improvements,
> > especially small performance improvements, not so much. Compatibility
> > bugs which break communications protocols must be fixed. Updates for
> > new versions of communication protocols and new ciphers, probably.
> >
> > There's a wide grey area in between, it's true.
> Yes, and that is where reviewers can and should be brought in to help
> clarify things.
> 
> regards,
> 
> 
> Andrew Dinn
> -----------
> Red Hat Distinguished Engineer
> Red Hat UK Ltd
> Registered in England and Wales under Company Registration No. 03798903
> Directors: Michael Cunningham, Michael ("Mike") O'Neill