I've also updated https://issues.apache.org/jira/browse/INFRA-16602 with some questions for pono!
-s On Tue, Jul 31, 2018 at 5:56 PM Sid Anand <san...@apache.org> wrote: > 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 <san...@apache.org> 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 <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 >>>> > > > >> >>>> > > > > >>>> > > > >>>> > > >>>> > >>>> >>>
