IMPORTANT REMINDER: 2 Reviews (ie approvals) are required for most client-libs changes

[Aleksei Ivanov]

In some cases, one reviewer may be enough… for some trivial changes, 
perhaps for some test-only updates.

Even in those cases, non-urgent PRs should remain open for *at least 24 
hours* after getting the first approval.

--
Alexey

On 13/07/2023 21:33, Philip Race wrote:
> That would be nice but I suspect it would not be easy to get agreement 
> on that.
>
> At least the dev guide also says "Some teams may require more Reviewers."
> https://openjdk.org/guide/#fixing-a-bug
>
> And it recommends 24 business hrs as well ..
>
> "In general all PRs should be open for at least 24 hours to allow for 
> reviewers in all time zones to get a chance to see it. It may actually 
> happen that even 24 hours isn’t enough. Take into account weekends, 
> holidays, and vacation times throughout the world and you’ll realize 
> that a change that requires more than just a trivial review may have 
> to be open for a while. In some areastrivial 
> <https://openjdk.org/guide/#trivial>changes are allowed to be pushed 
> without the 24 hour delay. Ask your reviewers if you think this 
> applies to your change."
>
> https://openjdk.org/guide/#life-of-a-pr
>
> -phil.
>
> On 7/13/23 1:24 PM, Thomas Stüfe wrote:
>> I think it would be less confusing all around to have a general 
>> requirement for 2 reviewers across the whole OpenJDK.
>>
>> On Thu, Jul 13, 2023 at 5:28 PM Philip Race <philip.race at oracle.com> 
>> wrote:
>>
>>     Please see "Code Reviews" on the Group page
>>     https://openjdk.org/groups/client-libs/ where it says
>>
>>     The Java Client Library Group has always standardized on two
>>     approvals - where at least one must have the Reviewer role.
>>     Historically this was addressed entirely by social conventions
>>     but today the tooling plays a role - and the JDK project is set
>>     up to mark a PR as ready for integration after a single approval
>>     by a person with the Reviewer role - which is not consistent with
>>     the Client Libraries policy.
>>     The tooling cannot automatically enforce this on a per-module
>>     basis and it is not reasonable to expect others to add
>>     "/reviewers 2" to every PR.
>>     The fixer therefore needs to understand the policies and wait for
>>     a second approval.
>>
>>     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>
>>     As an example of a PR about which there was zero urgency and
>>     should have had a 2nd approval see
>>
>>     https://github.com/openjdk/jdk/pull/14795
>>
>>     -phil.
>>
>