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

Kevin Rushforth kevin.rushforth at oracle.com
Mon Dec 16 15:38:00 UTC 2019


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



More information about the openjfx-dev mailing list