Changes to 8u and 11u process

[Andrew Haley]

On 8/6/19 6:11 PM, Aleksey Shipilev wrote:
> (was surprised why it was not discussed here before, because I have comments)
> 
> On 8/6/19 3:46 PM, Andrew Haley wrote:
>> Backports which have already been approved by Oracle for their
>> proprietary fork will no longer need approval for 8u- and 11u-open.
>> There are two reasons for this. Firstly, it's safe to assume that
>> Oracle have done their due diligence with regard to suitability.
>> Secondly, it's a waste of effort on the part of the maintainer.
> 
> I vehemently oppose this rule.

Sorry, I didn't think this would be so controversial or it would have
been clearly labeled as an RFC.

> First, this dismantles the consistency of the process ("every change
> should be approved, here is the checklist [1]") for the sake of less
> maintainer work (which can be alleviate by assigning more
> maintainers, as we did with Christoph). Having a single checklist
> for everything is simpler to remember, simpler to act upon, and it
> provides less cognitive load. We need to continue doing the same
> thing on all paths, and if on some of those paths it would reduce to
> mechanical rubber-stamping work, so be it. It is the same as with
> code reviews in trivial patches: we do not circumvent the review
> process by telling "trivial patches do not need review", we just say
> "trivial patches enjoy more mechanical rubber-stamping approach".

It's not about less maintainer work at all: there is little or no work
for the maintainer to do. It's that the process adds very little
information because approvals for clean test backports and backports
to maintain parity with Oracle will very probably automatically go
through. It does seem to me like process for process' sake in most
cases, where the programmer pushes a button and the maintainer pushes
a button in response.

But OK, I'm not trying to dictate this. If people here want to carry
on using this processI won't stand in your way.

> Second, summarily accepting every Oracle-proprietary change
> dismantles the maintainers' responsibility over everything in JDK
> Updates projects. There are changes that could be rejected from JDK
> Updates, even if Oracle-internal repositories have it: platforms not
> supported, features not maintained, cleanups not needed.

Maybe, but I'd be very surprised if such a patch survived review.

> We have to reserve the opportunity to act upon this, if/when
> needed. It feels like this rule should just go to maintainer's rule
> book: once somebody requests the backport for something
> Oracle-proprietary fork has, rubber-stamp it. But do not summarily
> relinquish maintainer's power to veto.

OK. As above, if people want to do so there's no very strong reason
not to make this advice to maintainers instead of a process reduction.

>> Backports of fixes to tests no longer need to be approved, but they
>> should still be reviewed if there are any changes.
> 
> Same thing as above.
> 
>> Where it makes sense, please minimize the size of the patches. If you
>> can adapt a backport patch to fit rather than also backporting a bunch
>> of its dependencies, please consider doing so. Bear in mind that
>> stability is the most important feature that these updates projects
>> have. Simple history in backports is important: it's more important to
>> reduce the volume of changes to 8u and 11u than it is to reduce the
>> differences between the backports and head.
> 
> Understanding that retrofitting the patch comes with the risks in
> itself, in many cases bringing up more code is better, if it
> minimizes the difference against other versions, applies
> mechanically, matches the history of the later code.

True, that can happen. This is a judgment call, that's why I said
"Where it makes sense," and I mean *only* where it makes sense, please
minimize the size of patches. If it clearly is better to backport a
bigger chunk of code, fair enough, but it should not always be a
reviewer's job to cut down patches.

> So, I think we should default to bringing in the patches wholesale,
> and leave it to maintainers/reviewers to ask backporters to cut the
> patches shorter if maintainers/reviewers argue there is no reason to
> bring large change with it.

OK, so we disagree about the default. I don't think that bringing in
extra code is less risky than adjusting patches, but it depends on the
actual patch, as above.

> It happened multiple times already, and I don't think it needs to be
> changed.

Yes, I know it has. It can go either way, but I don't want to see the
updates projects with a great deal of churn. Our work is all about
minimizing risk, and trying to keep the updates projects clean. When
it's less risky to backport more code than edit the patch, that's a
really good reason to do so. But both approaches carry some risk.

-- 
Andrew Haley  (he/him)
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