Do other projects also have multi-reviewers requirements? 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.
Not sure how complicated it would be to implement. - Nir On Mon, Dec 16, 2019 at 5:39 PM 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 > >>> > > > > > > > >
