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

[Philip Race]

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