Great, thank you Robert! Am Do., 31. Jan. 2019 um 16:44 Uhr schrieb Robert Metzger < rmetz...@apache.org>:
> Sorry, I haven't been active in the business of merging pull requests > recently. But you are both totally right: There is now a shiny big button > for merging pull requests. > > Regarding permissions for the bot: I'm going to ask Infra if the bot can > get permissions to label and merge pull requests. > > I have addressed most of Fabian's feature requests. I will now update the > website PR with the review guide and then activate the bot! > > > On Tue, Jan 29, 2019 at 1:13 PM Chesnay Schepler <ches...@apache.org> > wrote: > > > I assume he means that bots cannot do it, as they'd require committer > > permissions. Same with assigning reviewers and such. > > > > On 29.01.2019 13:09, Aljoscha Krettek wrote: > > > What do you mean by “merging cannot happen through the GitHub user > > interface”? You can in fact merge PRs by clicking on the merge button, or > > “rebase and merge”. > > > > > > Aljoscha > > > > > >> On 29. Jan 2019, at 11:58, Robert Metzger <rmetz...@apache.org> > wrote: > > >> > > >> @Fabian: Thank you for your suggestions. Multiple approvals in one > > comment > > >> are already possible. > > >> I will see if I can easily add multiple approvals in one line as well. > > >> I will also address 2) and 3). > > >> > > >> Regarding usage of the bot: Anyone can use it! It is up to the > committer > > >> who's merging in the end whether they are happy with the approver. > > >> One of the feature ideas I have is to indicate whether somebody is PMC > > or > > >> committer. > > >> > > >> I'm against enforcing the order of approvals for now. I fear that this > > will > > >> make the tool too restrictive. I like Ufuk's idea of putting a note > into > > >> the tracking comment for now. > > >> Once it's active and we are using it day to day, we'll probably learn > > what > > >> features we need the most. > > >> > > >> > > >> @Ufuk: I think we should put it into a Apache repo at some point. But > > I'm > > >> not sure if it's worth going through the effort of setting up a new > repo > > >> now, given that the bot is not even active yet, and we are not sure if > > it's > > >> going to be useful. > > >> Once it is active for a month or two, I will move it. > > >> > > >> Regarding the bots in general: I don't see a problem with having > > multiple > > >> bots in place, as long as they get along with each other well. > > >> We should try not to reinvent the wheel, if there's already a good bot > > >> implementation, I don't see a reason to not use it. > > >> The problem in our case is that we have limited access to our GitHub > > page, > > >> and merging can not happen through the GitHub user interface -- so I > > guess > > >> many "off the shelf" bots will not work in our environment. > > >> I'm thinking already about approaches how to automatically merge pull > > >> requests with the bot. But this will be a separate mailing list thread > > :) > > >> > > >> Thanks for the feedback! > > >> > > >> > > >> > > >> > > >> On Mon, Jan 28, 2019 at 5:20 PM Ufuk Celebi <u...@apache.org> wrote: > > >> > > >>> Thanks for the clarification. I agree that it only makes sense to > > >>> check the points in order. +1 to add this if we can think of a nice > > >>> way to do it. I'm not sure how we would enforce the order with the > bot > > >>> since there is only indirect feedback to a bot command. The only > thing > > >>> I can think of at the moment is to add a note to a check in case > > >>> earlier steps where not executed. Just ignoring a bot command because > > >>> other commands have not been executed before feels not helpful to me > > >>> (since we can't prevent reviewers to jump to later steps if they wish > > >>> to do so). > > >>> > > >>> I'd rather add a bold note to the bot template that makes clear that > > >>> all points should be followed in order to avoid potentially redundant > > >>> work. > > >>> > > >>> – Ufuk > > >>> > > >>> On Mon, Jan 28, 2019 at 5:01 PM Fabian Hueske <fhue...@gmail.com> > > wrote: > > >>>> The points in the review template are in the order in which they > > should > > >>> be > > >>>> checked, i.e., first checking the description, then consensus and > > finally > > >>>> checking the code. > > >>>> Currently, it is possible to tick off the code box before checking > the > > >>>> description. > > >>>> One motivation for the process was to do the steps in exactly the > > >>> proposed > > >>>> order for example to to avoid detailed code reviews before there was > > >>>> consensus whether the contribution was welcome or not. > > >>>> > > >>>> Am Mo., 28. Jan. 2019 um 16:54 Uhr schrieb Ufuk Celebi < > > u...@apache.org>: > > >>>> > > >>>>> 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 > > >>>>>>>>>>> > > > > > > > >
