airflow-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Sid Anand <san...@apache.org>
Subject Re: We've migrated to Github to repo!
Date Wed, 01 Aug 2018 01:02:44 GMT
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 <sanand@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 <sanand@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 <sanand@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 <sanand@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 <sanand@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
>>>> > > > >>
>>>> > > > >
>>>> > > >
>>>> > >
>>>> >
>>>>
>>>

Mime
  • Unnamed multipart/related (inline, None, 0 bytes)
View raw message