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