I've opened the following ticket to disable the "Merge Commits" option in the Merge Button, leaving the other 2 options available (e.g. Squash and Merge, Rebase and Merge).
https://issues.apache.org/jira/browse/INFRA-16852 -s On Tue, Jul 31, 2018 at 4:51 PM Sid Anand <[email protected]> wrote: > If squash & merge does a rebase under the hood, then I agree that is the > way to go. > > -s > > On Tue, Jul 31, 2018 at 4:50 PM Sid Anand <[email protected]> wrote: > >> So, in GitHub, we can disable certain options. So under settings --> >> options, there is a section called "Merge Button", with 3 options: "Allow >> merge commits", "Allow squash merging", and "Allow rebase merging". You can >> see this on your fork but not on the >> https://github.com/apache/incubator-airflow.git since we (committers) >> are not admins. >> >> The default option is "Allow merge commits", but as Fokko mentions, >> GitHub remembers your preference once you executed one of the merge >> strategies on your most recent PR merge. Maybe we should just file a ticket >> to disable "Allow merge commits" and just allow the other 2? >> >> [image: Screenshot 2018-07-31 16.37.14.png] >> >> >> Generally speaking, in the old apache hosted setup (i.e. >> >> https://git-wip-us.apache.org/repos/asf/incubator-airflow.git) , the >> master branch was not protected -- force push was allowed, so I'm guessing >> the same would be true here? In the old setup, that was not recommended for >> many obvious reasons and some not-so-obvious ones. The non-obvious one was >> that a force push to apache broke the mirroring to >> >> https://github.com/apache/incubator-airflow.git >> >> >> How does the new system work? Does it replicate from >> https://github.com/apache/incubator-airflow.git to >> https://git-wip-us.apache.org/repos/asf/incubator-airflow.git? >> >> >> The old system replicated in the other direction? I suspect we don't want >> some committers to use the old way and some to use the new way since the >> replication directions oppose one another. >> >> On Tue, Jul 31, 2018 at 12:50 PM Maxime Beauchemin < >> [email protected]> wrote: >> >>> What I meant by changing history is mutating one or many SHAs in the >>> branch, an operation that would require force-pushing, which merging >>> doesn't do. Personally I prefer "Squash & Merge" as it makes for a >>> merge-commit free `git log` and having a linear branch history in master >>> that aligns with when things were introduced to the branch. >>> >>> It's possible to disable some of these options from the repo (only if >>> you're an Admin, meaning we'd have to involve INFRA to change that). But >>> it's good to have options for the cases I mentioned above. >>> >>> So committers, use "Squash and Merge"! It matches our previous process >>> when >>> using the defaults in the now defunct `scripts/airflow-pr` >>> >>> [I'm really hoping I'm not starting a merge vs rebase workflow debate >>> here...] >>> >>> Max >>> >>> On Tue, Jul 31, 2018 at 12:37 PM Driesprong, Fokko <[email protected] >>> > >>> wrote: >>> >>> > Hi Max, >>> > >>> > You're right. I just started plowing though my mailbox and merged a >>> commit >>> > without squash and merge, but it changes history as you mention. >>> > Nice thing of Github is if you change it, it remembers your preference >>> > which is Squash and Merge :-) >>> > >>> > Love the Gitbox so far, great work! >>> > >>> > Cheers, Fokko >>> > >>> > 2018-07-31 21:34 GMT+02:00 Maxime Beauchemin < >>> [email protected]>: >>> > >>> > > "Squash & Merge" (the default) does the right thing (squashes the >>> > multiple >>> > > commit and replays the resulting commit on top of master), we should >>> use >>> > > that most of the times. We'd only want to merge if we wanted to >>> preserve >>> > > history from within the PR (multiple collaborators or multiple >>> important >>> > > commits that we want to keep detailed in master for instance). >>> > > >>> > > I'm not sure how to verify whether the `master` branch is protected >>> on >>> > this >>> > > setup (without pushing to it as a test, which I'd rather not do). We >>> > should >>> > > make sure that it is though as changing history on master can cause >>> all >>> > > sorts of problems. >>> > > >>> > > Max >>> > > >>> > > On Tue, Jul 31, 2018 at 9:21 AM Sid Anand <[email protected]> wrote: >>> > > >>> > > > The other benefit of using Option 3 over Option 1 is that you >>> maintain >>> > > the >>> > > > history of who committed and who authored in one line in the Git >>> log-- >>> > > i.e. >>> > > > "bob33 authored and ashb committed 3 hours ago" instead of just >>> "ashb >>> > > > committed" for a merge commit followed by the commit(s) from bob33. >>> > > > >>> > > > On Tue, Jul 31, 2018 at 9:11 AM Sid Anand <[email protected]> >>> wrote: >>> > > > >>> > > > > Ash, >>> > > > > This is pretty cool. I just merged one PR from GH directly. >>> > > > > >>> > > > > Interestingly, I still used the `dev/airflow-pr work_local` to >>> test >>> > out >>> > > > > the PR, but merging directly in the GitHub UI afterwards >>> definitely >>> > > > avoided >>> > > > > my needing to do another `dev/airflow-pr merge` CLI command. >>> > > > > >>> > > > > There are 3 options in the UI: The default is "Create a merge >>> commit" >>> > > > > (Option 1). I think the ones we want is the "Rebase & Merge" >>> (Option >>> > > 3), >>> > > > > which requires that PR submitters squash their commits. >>> Otherwise, we >>> > > > could >>> > > > > use "Squash & Merge" (Option 2), though I am not clear if Squash >>> & >>> > > Merge >>> > > > is >>> > > > > more like option 1 or option 3. >>> > > > > >>> > > > > -s >>> > > > > >>> > > > > On Mon, Jul 30, 2018 at 7:19 PM Andrew Phillips < >>> > [email protected] >>> > > > >>> > > > > wrote: >>> > > > > >>> > > > >> > We should ask Apache infra to send the GH notifs to another >>> > mailing >>> > > > >> > list. >>> > > > >> >>> > > > >> Over at jclouds, we created a "notifications@" list for this >>> > purpose >>> > > > >> (well, actually we renamed "issues@" to "notifications@"), and >>> send >>> > > > >> messages there: >>> > > > >> >>> > > > >> https://issues.apache.org/jira/browse/INFRA-7180 >>> > > > >> >>> https://mail-archives.apache.org/mod_mbox/jclouds-notifications/ >>> > > > >> >>> > > > >> Regards >>> > > > >> >>> > > > >> ap >>> > > > >> >>> > > > > >>> > > > >>> > > >>> > >>> >>
