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 < maximebeauche...@gmail.com> 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 <fo...@driesprong.frl> > 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 <maximebeauche...@gmail.com > >: > > > > > "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 <san...@apache.org> 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 <san...@apache.org> 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 < > > aphill...@qrmedia.com > > > > > > > > > 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 > > > > >> > > > > > > > > > > > > > > >
