You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@airflow.apache.org by Sid Anand <sa...@apache.org> on 2018/08/01 00:56:13 UTC

Re: We've migrated to Github to repo!

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 <sa...@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 <sa...@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 <
>> maximebeauchemin@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 <fokko@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 <
>>> maximebeauchemin@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 <sa...@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 <sa...@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 <
>>> > aphillips@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
>>> > > > >>
>>> > > > >
>>> > > >
>>> > >
>>> >
>>>
>>

Re: We've migrated to Github to repo!

Posted by Sid Anand <sa...@apache.org>.
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 <sa...@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 <sa...@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 <sa...@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 <
>>> maximebeauchemin@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 <fokko@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 <
>>>> maximebeauchemin@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 <sa...@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 <sa...@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 <
>>>> > aphillips@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
>>>> > > > >>
>>>> > > > >
>>>> > > >
>>>> > >
>>>> >
>>>>
>>>