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 <san...@apache.org> 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 < > 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 >> > > > >> >> > > > > >> > > > >> > > >> > >> >
