I like the idea of controlling squashing or not explicitly in the mergebot invocation. I don't think it needs to be made interactive, but just based on preparing the PR appropriately.
I propose this for the default `@asfgit merge`: Don't squash, but reject merges that have commits that are obviously intended to be squashed. Force the user to resolve that into exactly the final commits (after review is done). The use of this aspect of git probably means the user is advanced and this is not a burden. Commits that should be rejected: the standard "fixup!" or "squash!" prefixes (probably "fixup" and "squash" without punctuation) and maybe any commit message with "review" in it since we get a lot of commits like "respond to review comments". For `@asfgit merge squash` (non-default): still reject "fixup!" and "squash!" commits because those are instructions that might result in reordering and conflicts. Instead, target just those PRs that are a sequence of rambling commits added for review, and always squash into the first. Kenn On Tue, Jul 11, 2017 at 8:07 AM, Ismaël Mejía <ieme...@gmail.com> wrote: > Thanks a lot Jason, > > Great that Infra solved (2) so fast. > > About (3), maybe the extra pause/validation is not needed, because the > bot will in principle make its work appropriately, maybe what we could > just have is a way to see the git branch with the commits that > mergebot will do as part of the review process similar to the view we > have for the staging website, just as an extra validation but in the > same step. > > About the squashing parameter I think it is a good idea, but I am > afraid that this ends up becoming more complex/interactive because > squashing can imply to rewrite the commit message so I am not sure if > this achievable (or worth the effort). > > WDYT ? > > On Tue, Jul 11, 2017 at 10:37 AM, Aljoscha Krettek <aljos...@apache.org> > wrote: > > +1 > > > > This is excellent! > > > >> On 10. Jul 2017, at 21:42, Jason Kuster <jasonkus...@google.com.INVALID> > wrote: > >> > >> (quick update re #2 above): ~4 minutes after I reopened the ticket, it's > >> fixed. > >> https://github.com/apache/infrastructure-puppet/commit/ > 709944291da5e8aea711cb8578f0594deb45e222 > >> updates the website to the correct address. Infra is once again the > best. > >> > >> On Mon, Jul 10, 2017 at 12:38 PM, Jason Kuster <jasonkus...@google.com> > >> wrote: > >> > >>> Glad to hear everyone's pretty happy about it! Have a couple answers > for > >>> your questions. > >>> > >>> Ted: I believe the MFA stuff (two-factor auth on github) is necessary > for > >>> getting the additional features on GitHub (reviewer, etc), but may not > be > >>> necessary for MergeBot. I'll check in with Infra and get back to you. > >>> > >>> Ismaël: Great questions! Answered below. > >>> > >>> 1. The code will likely be transitioned over to an Infra-controlled > >>> repository, but for now is under my account: https://github.com/ > >>> jasonkuster/merge-bot. It's written in Python, so Python aficionados > >>> especially feel free to take a look, kick the tires, and open PRs. > >>> > >>> 2. Glad to hear mergebot worked for you. :) The website not showing > >>> appears to be an issue with transitioning to GitBox; it seems a > reference > >>> may have not been updated. Thanks for the report! I've reopened > >>> https://issues.apache.org/jira/browse/INFRA-14405 to track. > >>> > >>> 3. I'd love to chat about this more! It's totally possible to have > >>> mergebot pause and show the status of the repository before it does the > >>> final push, but given that mergebot is merging PRs serially I don't > want to > >>> have someone forget to click "ok" and block other people's PRs. One > other > >>> option would be to allow the person requesting the merge to say > something > >>> like "@asfgit merge squash" or "@asfgit merge nosquash", parametrizing > the > >>> merge request. Thoughts? > >>> > >>> On Mon, Jul 10, 2017 at 10:52 AM, Mark Liu <mark...@google.com.invalid > > > >>> wrote: > >>> > >>>> +1 Awesome work! > >>>> > >>>> Thank you Jason!!! > >>>> > >>>> Mark > >>>> > >>>> On Mon, Jul 10, 2017 at 10:05 AM, Robert Bradshaw < > >>>> rober...@google.com.invalid> wrote: > >>>> > >>>>> +1, this is great! I'll second Ismaël's list requests, especially 1 > and > >>>> 3. > >>>>> > >>>>> On Mon, Jul 10, 2017 at 2:09 AM, Ismaël Mejía <ieme...@gmail.com> > >>>> wrote: > >>>>>> Excellent!, Automation of such repetitive (and error-prone) tasks is > >>>>>> strongly welcomed. > >>>>>> > >>>>>> Thanks for making this happen Jason! > >>>>>> > >>>>>> Some comments: > >>>>>> > >>>>>> 1. I suppose the code of mergebot is now part of Apache Infra, no? > Do > >>>>>> you know exactly where the code is hosted? And what is the procedure > >>>>>> in case somebody wants to improve it or change something in the > >>>>>> future? I suppose other projects can/would benefit of this. > >>>>>> > >>>>>> 2. I configured and used the mergebot with success, however the > >>>>>> website does not reflect the changes of the PR I 'merged', I suppose > >>>>>> there are still some things we have to fix, because the changes are > >>>>>> not there. > >>>>>> (The PR I am talking about is https://github.com/apache/ > >>>>> beam-site/pull/264) > >>>>>> > >>>>>> 3. Other thing I noticed is that the mergebot didn’t squash the > >>>>>> commits (this probably makes sense) and I didn’t realize this to do > it > >>>>>> before because there is not a preview of the state of the actions > that > >>>>>> the mergebot is going to do, can this eventually be improved? (I > don’t > >>>>>> know if this makes sense because this will add an extra validation > >>>>>> step and we must trust robots anyway :P). > >>>>>> > >>>>>> This new issue is something that reviewers/committers must remember, > >>>>>> and talking about this we need to update this in the contribution > >>>>>> guide to include the configuration/use of the mergebot instructions. > >>>>>> > >>>>>> Thanks again Jason and the other who made this possible, this is > >>>> great! > >>>>>> Ismaël > >>>>>> > >>>>>> ps. I’m eager to see this included too for the beam project. > >>>>>> > >>>>>> On Sat, Jul 8, 2017 at 7:28 AM, tarush grover < > >>>> tarushappt...@gmail.com> > >>>>> wrote: > >>>>>>> This is really good!! > >>>>>>> > >>>>>>> Regards, > >>>>>>> Tarush > >>>>>>> > >>>>>>> On Sat, 8 Jul 2017 at 10:20 AM, Jean-Baptiste Onofré < > >>>> j...@nanthrax.net> > >>>>>>> wrote: > >>>>>>> > >>>>>>>> That's awesome ! > >>>>>>>> > >>>>>>>> Thanks Jason ! > >>>>>>>> > >>>>>>>> Regards > >>>>>>>> JB > >>>>>>>> > >>>>>>>> On 07/07/2017 10:21 PM, Jason Kuster wrote: > >>>>>>>>> Hi Beam Community, > >>>>>>>>> > >>>>>>>>> Early on in the project, we had a number of discussions about > >>>>> creating an > >>>>>>>>> automated tool for merging pull requests. I’m happy to announce > >>>> that > >>>>>>>> we’ve > >>>>>>>>> developed such a tool and it is ready for experimental usage in > >>>> Beam! > >>>>>>>>> > >>>>>>>>> The tool, MergeBot, works in conjunction with ASF’s existing > >>>> GitBox > >>>>> tool, > >>>>>>>>> providing numerous benefits: > >>>>>>>>> * Automating the merge process -- instead of many manual steps > >>>> with > >>>>>>>>> multiple Git remotes, merging is as simple as commenting a > >>>> specific > >>>>>>>> command > >>>>>>>>> in GitHub. > >>>>>>>>> * Automatic verification of each pull request against the latest > >>>>> master > >>>>>>>>> code before merge. > >>>>>>>>> * Merge queue enforces an ordering of pull requests, which > ensures > >>>>> that > >>>>>>>>> pull requests that have bad interactions don’t get merged at the > >>>> same > >>>>>>>> time. > >>>>>>>>> * GitBox-enabled features such as reviewers, assignees, and > >>>> labels. > >>>>>>>>> * Enabling enhanced use of tools like reviewable.io. > >>>>>>>>> > >>>>>>>>> If you are a committer, the first step is to link your Apache and > >>>>> GitHub > >>>>>>>>> accounts at http://gitbox.apache.org/setup. Once the accounts > are > >>>>>>>> linked, > >>>>>>>>> you should have immediate access to new GitHub features like > >>>> labels, > >>>>>>>>> assignees, etc., as well as the ability to merge pull requests by > >>>>> simply > >>>>>>>>> commenting “@asfgit merge” on the pull request. MergeBot will > >>>>> communicate > >>>>>>>>> its status back to you via the same mechanism used already by > >>>>> Jenkins. > >>>>>>>>> > >>>>>>>>> This functionally is currently enabled for the “beam-site” > >>>> repository > >>>>>>>> only. > >>>>>>>>> In this phase, we’d like to gather feedback and improve the user > >>>>>>>> experience > >>>>>>>>> -- so please comment back early and often. Once we are happy with > >>>> the > >>>>>>>>> experience, we’ll deploy it on the main Beam repository, and > >>>>> recommend it > >>>>>>>>> for wider adoption. > >>>>>>>>> > >>>>>>>>> I’d like to give a huge thank you to the Apache Infrastructure > >>>> team, > >>>>>>>>> especially Daniel Pono Takamori, Daniel Gruno, and Chris > >>>>> Thistlethwaite > >>>>>>>> who > >>>>>>>>> were instrumental in bringing this project to fruition. > >>>> Additionally, > >>>>>>>> this > >>>>>>>>> could not have happened without the extensive work Davor put in > to > >>>>> keep > >>>>>>>>> things moving along. Thank you Davor. > >>>>>>>>> > >>>>>>>>> Looking forward to hearing your comments and feedback. Thanks. > >>>>>>>>> > >>>>>>>>> Jason > >>>>>>>>> > >>>>>>>> > >>>>>>>> -- > >>>>>>>> Jean-Baptiste Onofré > >>>>>>>> jbono...@apache.org > >>>>>>>> http://blog.nanthrax.net > >>>>>>>> Talend - http://www.talend.com > >>>>>>>> > >>>>> > >>>> > >>> > >>> > >>> > >>> -- > >>> ------- > >>> Jason Kuster > >>> Apache Beam / Google Cloud Dataflow > >>> > >> > >> > >> > >> -- > >> ------- > >> Jason Kuster > >> Apache Beam / Google Cloud Dataflow > > >
