So is there some “review complete” or “review closed” or some following state available for the bot to key on maybe instead of reacting to one reviewer completion?
Eric Bresie ebre...@gmail.com > On December 16, 2019 at 9:38:00 AM CST, Kevin Rushforth > <kevin.rushfo...@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.rushfo...@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 > > > > > > > > > > >
