osdir.com


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

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


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