Hi Robert, Thanks for working on the bot! I have a few suggestions / questions:
Suggestions: 1) It would be great to approve multiple boxes in one comment. Either as > @flinkbot approve contribution consensus or by > @flinkbot approve contribution > @flinkbot approve consensus 2) Extend the last line of the description by adding something like "See Pull Request Review Guide for how to use the Flink bot."? 3) Rephrase the first line to include "[description]" instead of "[contribution]", as it is about approving the description Questions: 1) Can anybody use the bot or just committers? 2) Does it make sense to enforce the order in which approvals are given? Best, Fabian Am Mi., 23. Jan. 2019 um 13:51 Uhr schrieb Robert Metzger < rmetz...@apache.org>: > I have the bot now running in https://github.com/flinkbot/test-repo/pulls > Feel free to play with it. > > On Wed, Jan 23, 2019 at 10:25 AM Robert Metzger <rmetz...@apache.org> > wrote: > > > Okay, cool! I'll let you know when the bot is ready in a test repo. > > While you (and others) are testing it, I'll open a PR for the docs. > > > > On Wed, Jan 23, 2019 at 10:15 AM Fabian Hueske <fhue...@gmail.com> > wrote: > > > >> Oh, that's great news! > >> In that case we can just close the PR and start with the bot right away. > >> I think it would be good to extend the PR Review guide [1] with a > section > >> about the bot and how to use it. > >> > >> Fabian > >> > >> [1] https://flink.apache.org/reviewing-prs.html > >> > >> Am Mi., 23. Jan. 2019 um 10:03 Uhr schrieb Robert Metzger < > >> rmetz...@apache.org>: > >> > >> > Hey, > >> > > >> > as I've mentioned already in the pull request, I have started > >> implementing > >> > a little bot for GitHub that tracks the checklist [1] > >> > The bot is monitoring incoming pull requests. It creates a comment > with > >> the > >> > checklist. > >> > Reviewers can write a message to the bot (such as "@flinkbot approve > >> > contribution"), then the bot will update the checklist comment. > >> > > >> > As an upcoming feature, I also plan to add a label to the pull request > >> when > >> > all the checklist conditions have been met. > >> > > >> > I hope to finish the bot today. After some initial testing, we can > >> deploy > >> > it with Flink (if there are no objections by the community). > >> > > >> > > >> > [1] https://github.com/rmetzger/flink-community-tools > >> > > >> > > >> > On Tue, Jan 22, 2019 at 3:48 PM Fabian Hueske <fhue...@gmail.com> > >> wrote: > >> > > >> > > Hi everyone, > >> > > > >> > > A few months ago the community discussed and agreed to improve the > PR > >> > > review process [1-4]. > >> > > The idea is to follow a checklist to avoid in-depth reviews on > >> > > contributions that might not be accepted for other reasons. Thereby, > >> > > reviewers and contributors do not spend their time on PRs that will > >> not > >> > be > >> > > merged. > >> > > The checklist consists of five points: > >> > > > >> > > 1. The contribution is well-described. > >> > > 2. There is consensus that the contribution should go into to Flink. > >> > > 3. [Does not need specific attention | Needs specific attention for > X > >> | > >> > Has > >> > > attention for X by Y] > >> > > 4. The architectural approach is sound. > >> > > 5. Overall code quality is good. > >> > > > >> > > Back then we added a review guide to the website [5] but did not put > >> the > >> > > new process in place yet. I would like to start this now. > >> > > There is a PR [6] that adds the review checklist to the PR template. > >> > > Committers who review add PR should follow the checklist and tick > and > >> > sign > >> > > off the boxes by updating the PR description. For that committers > >> need to > >> > > be members of the ASF Github organization. > >> > > > >> > > If nobody has concerns, I'll merge the PR in a few days. > >> > > Once the PR is merged, the reviews of all new PRs should follow the > >> > > checklist. > >> > > > >> > > Best, > >> > > Fabian > >> > > > >> > > [1] > >> > > > >> > > > >> > > >> > https://lists.apache.org/thread.html/dcbe377eb477b531f49c462e90d8b1e50e0ff33c6efd296081c6934d@%3Cdev.flink.apache.org%3E > >> > > [2] > >> > > > >> > > > >> > > >> > https://lists.apache.org/thread.html/172aa6d12ed442ea4da9ed2a72fe0894c9be7408fb2e1b7b50dfcb8c@%3Cdev.flink.apache.org%3E > >> > > [3] > >> > > > >> > > > >> > > >> > https://lists.apache.org/thread.html/5e07c1be8078dd7b89d93c67b71defacff137f3df56ccf4adb04b4d7@%3Cdev.flink.apache.org%3E > >> > > [4] > >> > > > >> > > > >> > > >> > https://lists.apache.org/thread.html/d7fd1fe45949f7c706142c62de85d246c7f6a1485a186fd3e9dced01@%3Cdev.flink.apache.org%3E > >> > > [5] https://flink.apache.org/reviewing-prs.html > >> > > [6] https://github.com/apache/flink/pull/6873 > >> > > > >> > > >> > > >
