Hi, I'm slightly prefer the bot option. The bot can post the review template automatically. But I do agree that we can start with a low-tech solution and add a bot later if find it helpful.
Best, Hequn On Wed, Oct 17, 2018 at 11:17 AM Jin Sun <isun...@gmail.com> wrote: > +1 > > On Tue, Oct 16, 2018 at 7:51 PM Tzu-Li Chen <wander4...@gmail.com> wrote: > > > Hi Fabian, > > > > +1 for your proposal. > > > > For the downside, I think after adding the review process template, > > the pull request template would be high level into 3 parts: > > > > 1. Greeting and community guiding. > > 2. User completed template. > > 3. Reviewer complete template. > > > > If we can visually separate them, i.e., help a new contributor regard the > > whole template into 3 parts, I think this downside is not so critical. > For > > some previous attempt, see also [1]. > > > > Best, > > tison. > > > > [1] https://github.com/apache/flink/pull/6722 > > > > > > vino yang <yanghua1...@gmail.com> 于2018年10月17日周三 上午9:57写道： > > > > > +1, > > > > > > Agree to use the PR template. > > > > > > Fabian Hueske <fhue...@gmail.com> 于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 < > > > > fhue...@gmail.com > > > > >: > > > > > > > > > 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 < > > > > > yanghua1...@gmail.com>: > > > > > > > > > >> 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 <aljos...@apache.org> 于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 <yanghua1...@gmail.com> > > > 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 <isun...@gmail.com> 于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 < > se...@apache.org> > > > > >> 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? > > > > >> > >> > > > > >> > >> > > > > >> > > > > > >> > > > > > >> > > > > > > > > > > > > > > > > > -- > Thanks, > Jin SUN >
