osdir.com


[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [DISCUSS] [Contributing] (3) - Review Tooling


+1,

Agree to use the PR template.

Fabian Hueske <fhueske@xxxxxxxxx> 于2018年10月17日周三 上午12:48写道:

> Hi everyone,
>
> Instead of manually adding the review progress template as a comment to
> every new PR, we could also append it to the PR description template.
> The benefits would be:
> + no need to add it manually since it is automatically added to each PR
> + the template is versioned in the Flink Git repository
> + contributors can learn about the review process before opening a PR
>
> On the downside, the template grows a bit at the end.
>
> What do you think?
>
> Best, Fabian
>
> Am Mo., 24. Sep. 2018 um 15:51 Uhr schrieb Fabian Hueske <
> fhueske@xxxxxxxxx
> >:
>
> > Hi,
> >
> > Coming back to the original topic of the thread: How to implement the
> > guided review process.
> >
> > I am in favor of starting with a low-tech solution.
> > We design a review template with a checkbox for the five questions (see
> > [1]) and a link to the detailed description of the review process ([1]
> will
> > be added to flink.apache.org).
> > Once a PR is opened, anybody (the PR author, a committer, any reviewer,
> > ...) can post the review template as a comment. Ideally this happens
> > shortly after the PR was opened.
> > If we find it necessary, we can later add a bot to automate posting the
> > template as comment.
> >
> > Once the template is posted, the PR can be reviewed by following the
> > process and answering the template questions.
> > When all boxes are ticked off, the PR can be merged.
> >
> > Best,
> > Fabian
> >
> >
> >
> >
> >
> > [1]
> >
> https://docs.google.com/document/d/1yaX2b9LNh-6LxrAmE23U3D2cRbocGlGKCYnvJd9lVhk/
> >
> >
> > Am Mo., 24. Sep. 2018 um 12:27 Uhr schrieb vino yang <
> > yanghua1127@xxxxxxxxx>:
> >
> >> Hi,
> >>
> >> About "valuable", I agree with @Aljoscha that there is no clear standard
> >> of
> >> judgment about "valuable".
> >> But I think the priority may be a more specific indicator, because the
> >> JIRA
> >> issue also has a "Priority" attribute.
> >> Maybe we can tag the PR, for example: use the "Label" function of
> github,
> >> or add the "[Priority]" tag to the PR title?
> >>
> >> Regarding the closure of inactive PR, I feel that it is more cautious to
> >> shut down artificially.
> >> Whether it is possible to explicitly assign a PR to a committer familiar
> >> with the module, which will reduce the unnecessary ping operation of
> many
> >> contributors.
> >> Because some people don't know which committer is familiar with the
> module
> >> he changed.
> >>
> >> Thanks, vino.
> >>
> >> Aljoscha Krettek <aljoscha@xxxxxxxxxx> 于2018年9月24日周一 下午5:03写道:
> >>
> >> > In Beam, we have a bot that regularly nags people about inactive PRs
> and
> >> > also closes them after long inactivity.
> >> >
> >> > And we use the github feature for assigning reviewers in github.
> >> >
> >> > Sometimes it is hard for people to judge how "valuable" a PR is. Maybe
> >> > some knowledgable people could mark PRs as valuable if they think
> it's a
> >> > good addition but if they don't have the review bandwith. Other people
> >> can
> >> > then search for valuable PRs that don't yet a reviewer and
> review/merge
> >> > them.
> >> >
> >> > Aljoscha
> >> >
> >> > > On 22. Sep 2018, at 04:25, vino yang <yanghua1127@xxxxxxxxx> wrote:
> >> > >
> >> > > Hi Jin Sun,
> >> > >
> >> > > Earlier this year, I also had these questions when I started
> >> contributing
> >> > > code to Flink. In fact, the timing of a PR being reviewed will be
> >> related
> >> > > to the priority of the problem solved by the PR.
> >> > > And when you indicate the module to which it belongs in the title of
> >> the
> >> > > PR, like "[FLINK-xxxx][module] XXXX", the person in charge of the
> >> > relevant
> >> > > module or the contributor who is familiar with it will find it
> easier.
> >> > >
> >> > > To Stephan:
> >> > >
> >> > > Maybe we can open a separate mail thread (after all, the current
> >> > discussion
> >> > > thread is about a specific topic) to hear the contributors about PR
> >> > review
> >> > > related questions and doubts. Perhaps some of their feedback will
> help
> >> > the
> >> > > community improve the way they review.
> >> > >
> >> > > Thanks, vino.
> >> > >
> >> > > Jin Sun <isunjin@xxxxxxxxx> 于2018年9月22日周六 上午6:40写道:
> >> > >
> >> > >> As a new contributor I cared about how to make my contribution
> >> accepted
> >> > by
> >> > >> the community, some questions:
> >> > >> 1) When will it get reviewed? Is there a rule about review
> timeline?
> >> > >> 2) There are long backlog of pull requests, What happened if a pull
> >> > >> request not get noticed, do we have some mechanism to make it
> moving
> >> > >> forward, like a pull request will be assigned a owner of reviewer?
> >> Or we
> >> > >> have a review queue and a pull request will be get handled fairly.
> >> > >>
> >> > >> Jin
> >> > >>
> >> > >>
> >> > >>> On Sep 20, 2018, at 12:56 PM, Stephan Ewen <sewen@xxxxxxxxxx>
> >> wrote:
> >> > >>>
> >> > >>> Hi all!
> >> > >>>
> >> > >>> This thread is dedicated to discuss the tooling we want to use for
> >> the
> >> > >>> reviews.
> >> > >>> It is spun out of the proposal *"A more structured approach to
> >> reviews
> >> > >> and
> >> > >>> contributions".*
> >> > >>>
> >> > >>>
> >> > >>> *Suggestions brought up so far*
> >> > >>>
> >> > >>>
> >> > >>> *Use comments / template with checklist*
> >> > >>>
> >> > >>> - Easy to do
> >> > >>> - Manual, a bit of reviewer overhead, reviewers needs to know the
> >> > >> process
> >> > >>>
> >> > >>> *Use a bot *
> >> > >>>
> >> > >>> - Automatically add the review questions to each new PR
> >> > >>> - Further details?
> >> > >>>
> >> > >>> *Use GitHub labels*
> >> > >>>
> >> > >>> - Searchable
> >> > >>> - possibly not obvious to new contributors
> >> > >>> - Any restrictions? Do members need to apply at ASF infra to have
> >> > >>> permissions to edit github labels?
> >> > >>
> >> > >>
> >> >
> >> >
> >>
> >
>