Skara - bot sending can-be-integrated message prematurely?
Eric Bresie
ebresiedev at gmail.com
Mon Dec 16 16:07:44 UTC 2019
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
Ebresie at gmail.com
> On December 16, 2019 at 9:38:00 AM CST, 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
> > > >
> >
> >
> >
>
More information about the openjfx-dev
mailing list