Skara - bot sending can-be-integrated message prematurely?

[Phil Race]

It would have to allow anyone who has reviewer status to add that comment.
Not just the author since if they knew we would have less need for it.

-Phil.

> On Dec 18, 2019, at 11:31 AM, Kevin Rushforth <kevin.rushforth at oracle.com> wrote:
> 
> That's an interesting idea. It would, of course, need to disallow reducing the number below the minimum specified in .jcheck/conf (e.g., we wouldn't allow "/reviewers 0").
> 
> -- Kevin
> 
> 
>> On 12/18/2019 10:36 AM, Nir Lisker wrote:
>>> The client libraries in the OpenJDK do as a default rule, excusing simple fixes.
>> 
>> Then maybe it would be helpful to have a "/reviewers n" command that will tell the bot how many reviewers are needed. It's less convoluted than the CSR tracking and basically replaces the comment a reviewer would make anyway to inform the PR author how many reviewers they should wait for. Extending the bot to look for n reviewers instead of the current 1 should not be far fetched.
>> 
>>  
>>> 
>> 
>>> On Wed, Dec 18, 2019 at 4:02 AM Philip Race <philip.race at oracle.com> wrote:
>>> 
>>> 
>>> On 12/16/19, 4:14 PM, Nir Lisker wrote:
>>> > Do other projects also have multi-reviewers requirements?
>>> 
>>> The client libraries in the OpenJDK do as a default rule, excusing 
>>> simple fixes.
>>> 
>>> >
>>> > I would think that at least for a CSR, which is OpenJDK-wide, a request
>>> > could be put in with the Skara to track it. A Reviewer (or Committer?)
>>> > could invoke a /csr command which will require the PR author to provide a
>>> > link to the CSR, and check that it was approved as part of the patch
>>> > approval process.
>>> 
>>> I think that if there is a CSR sub-task in JBS skara can key off whether 
>>> that is approved.
>>> This does mean skara needs to probe JBS but SFAIK its doing that a 
>>> hundred times
>>> a minute anyway.
>>> 
>>> -phil.
>>> 
>>> >
>>> > Not sure how complicated it would be to implement.
>>> >
>>> > - Nir
>>> >
>>> > On Mon, Dec 16, 2019 at 5:39 PM Kevin Rushforth<kevin.rushforth at oracle.com>
>>> > wrote:
>>> >
>>> >> That's a good question about whether we can ask for a slight rewording
>>> >> of the Skara bot message to indicate that there might be other things to
>>> >> check before "/integrate". I'll raise that question with the Skara team.
>>> >>
>>> >> As an aside, I noticed the other day that you typed "/integrate" after a
>>> >> single review, but in that case, there was no clear indication from Ajit
>>> >> (the first reviewer and the sponsor) that a second review was required,
>>> >> so I didn't comment on it.
>>> >>
>>> >> -- Kevin
>>> >>
>>> >>
>>> >> On 12/16/2019 6:41 AM, Jeanette Winzenburg wrote:
>>> >>> Kevin,
>>> >>>
>>> >>> thanks for the clarification :) My bad that I didn't re-read the
>>> >>> contrib.md. But then, who does? The lazy like myself do it
>>> >>> occasionally only (down to once and then forget about it<g>)
>>> >>>
>>> >>> Maybe the bot message can be improved? With some indication that its
>>> >>> (the bot's) knowledge about review requirements is limited, so would
>>> >>> require a careful check by the contributor before actually post the
>>> >>> /integrate comment? Actually, I think I goofed the other day, was
>>> >>> safed only by Ajit who waited for the 2nd review before his /sponsor.
>>> >>>
>>> >>> -- Jeanette
>>> >>>
>>> >>> Zitat von Kevin Rushforth<kevin.rushforth at oracle.com>:
>>> >>>
>>> >>>> I added a comment to the two PRs in question, but it bears discussion
>>> >>>> here.
>>> >>>>
>>> >>>> The Skara bot can't know whether all criteria have been met. It
>>> >>>> can't, for example, know whether there are outstanding comments from
>>> >>>> some reviewers that need to be addressed. Nor does it know which PRs
>>> >>>> need two reviewers (or sometimes a third if there is a specific
>>> >>>> person we would like to review it), which ones need a CSR, etc.
>>> >>>>
>>> >>>> So having it state authoritatively that the PR is ready to integrate
>>> >>>> is a bit misleading. This is documented in the Code Review section of
>>> >>>> the CONTRIBUTING [1] doc:
>>> >>>>
>>> >>>>> NOTE: while the Skara tooling will indicate that the PR is ready to
>>> >>>>> integrate once the first reviewer with a "Reviewer" role in the
>>> >>>>> project has approved it, this may or may not be sufficient depending
>>> >>>>> on the type of fix. For example, you must wait for a second approval
>>> >>>>> for enhancements or high-impact bug fixes.
>>> >>>> If anyone can think of a way to improve this and make it more clear,
>>> >>>> that would be helpful.
>>> >>>>
>>> >>>> -- Kevin
>>> >>>>
>>> >>>> [1] https://github.com/openjdk/jfx/blob/master/CONTRIBUTING.md
>>> >>>>
>>> >>>>
>>> >>>> On 12/16/2019 4:23 AM, Jeanette Winzenburg wrote:
>>> >>>>> Looks like it assumes a pull request as properly reviewed as soon as
>>> >>>>> it gets a single approve - independent on how many reviewers are
>>> >>>>> required, see f.i.
>>> >>>>>
>>> >>>>> https://github.com/openjdk/jfx/pull/15#issuecomment-565964995
>>> >>>>> https://github.com/openjdk/jfx/pull/6#issuecomment-566028296
>>> >>>>>
>>> >>>>> -- Jeanette
>>> >>>>>
>>> >>>
>>> >>>
>>> >>
>