I played around with the bot and it works pretty well. :-) @Robert: Are there any plans to contribute the code for the bot to Apache (potentially in another repository)?
I like Fabians suggestions. Regarding the questions: 1) I would make that dependent on whether you expected the review guideline to only apply to committers or also regular contributors. Since the bot is not merging PRs, I don't see a down side in having it open for all contributors (except potential noise). 2) What do you mean with "order of approvals"? Since there is another lively discussion going on about a bot for stale PRs, I'm wondering what the future plans for @flinkbot are. Do we want to have multiple bots for the project? Or do you plan to support staleness checks in the future? What about merging of PRs? – Ufuk On Mon, Jan 28, 2019 at 4:23 PM Fabian Hueske <fhue...@gmail.com> wrote: > > 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 > > >> > > > > >> > > > >> > > > > >
