jdk11u-dev is now switched to 11.0.5

[Andrew Haley]

On 6/4/19 8:40 PM, Andrew John Hughes wrote:
> 
> 
> On 03/06/2019 18:40, Andrew Haley wrote:
>> On 6/3/19 3:49 PM, Andrew John Hughes wrote:
>>>
> 
>>
>>> snip...
>>>
>>>>
>>>> That imposes a strict ordering: review first, then approval. I think
>>>> that approval and review can run in parallel: we don't want to fully
>>>> review every backport patch twice.
>>>
>>> The Oracle process I'm familiar with is sequential:
>>>
>>> "Additionally the comment should note whether the patch from the JDK
>>> Project applies cleanly. If not, the fix MUST be codereviewed in public
>>> and a public url to that review MUST be provided in the comment." [2]
>>>
>>> Under 8u, the approval e-mail used to refer to the review thread.
>>>
>>> I'm not suggesting reviewing every backport twice, but I also don't see
>>> how you can approve something if you don't know what you're approving.
>>
>> Maybe, but the patch itself has already been reviewed: all you're approving
>> is an already-reviewed patch. So maybe the patch itself is interesting, but
>> maybe not. The question for the approver is whether this patch is worth doing,
>> not whether it's a good patch: we already know it is.
> 
> If the operations are proceeding sequentially, then yes, the patch has
> already been reviewed.
> 
> If you allow approval to take place concurrently, then the review
> process may still be in progress or may not yet have started.
> 
> To what extent is approval based on the idea and to what extent on the
> implementation?

If it's reviewing the implementation, that's a second review. Which is
fine if that's what we want do do.

> A concurrent approval process works if it is just approving the
> idea, but, in reality, it is the approval of a specific
> implementation which may differ from the version previously applied
> to other JDK versions and have different consequences in this new
> context.
> 
> As examples, the backporting process may have to make significant
> alterations to the code changes, so as not to alter public API, or to
> the build & test changes, due to differences in those systems. The
> viability of those additional changes depends on the review of the
> backport, not the original patch.

Again, I say: that's what the review process is for.

>>> Keeping them sequential simplifies the process, because any fix that is
>>> approved has then also been reviewed, and so is only waiting on push.
>>>
>>> If the review begins or continues after approval, you end up with
>>> approved bugs that are not yet ready for push. This has been causing
>>> confusion for me with our "Approved requests without push" filter [3],
>>> with bugs listed in there that weren't even posted for review at one point.
>>
>> That's a tooling problem IMO.
>>
>>> I do suspect my perspective may be clouded by the fact I end up doing
>>> both review & approval for most 8u patches.
>>
>> I think so. These are different tasks,and having one person doing both
>> at once, essential as a single job, makes a nonsense of the
>> separation.
> 
> This may be true, but I don't see an easy solution to either.

Neither do I, but it seems pretty clear to me that the approval
process in this case is not actually happening. There's a review, the
approval box is ticked, and in it goes.

Having said that: is there a problem to solve? Are we allowing broken
or inappropriate patches through? I don't think so. If not, we're
merely worrying about correctly following a process rather than
ensuring the quality of OpenJDK.

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