mxnet-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Naveen Swamy <mnnav...@gmail.com>
Subject Re: Which merge option to use on the Import Julia binding PR?
Date Fri, 28 Sep 2018 21:00:18 GMT
Should we try to first being into a branch and then try merge that branch? 

> On Sep 28, 2018, at 4:40 PM, Pedro Larroy <pedro.larroy.lists@gmail.com> wrote:
> 
> I'm not familiar with the specifics of this contribution, as a general
> approach my understanding is that if the list of commits is big and you
> want to preserve history, usually merging is better so you keep history and
> causality, if you rebase all the commits on top of master you are changing
> the history of these commits which can't be individually reverted as some
> have suggested before. Maybe is because I come from a mercurial background,
> but my initial impression would be either to:
> 1. squash everything and rebase
> 2. or merge without rebasing or squashing.
> 
> Pedro.
> 
>> On Thu, Sep 27, 2018 at 3:10 PM Carin Meier <carinmeier@gmail.com> wrote:
>> 
>> Thanks everyone for the input. I'll try to summarize the feedback from the
>> responses:
>> 
>> Using Squash-Merge is the project standard for very good reasons. However,
>> in the case of this PR to bring in the Julia language from its sibling
>> repo, we want to preserve all the individual commits of the many
>> contributors that have worked over multiple years to make this a great
>> language binding. We will use Rebase-Merge for it.
>> 
>> Chiyuan - thanks for the suggestion of using a tag. I think we can try it
>> initially without it since there are other ways to browse the commit
>> history, like looking at the PRs. But, we can add the tag retroactively if
>> people start having trouble.
>> 
>> If there no objections, I will merge the PR using the above method in my
>> morning (EST).
>> 
>> Thanks everyone! I'm looking forward to having the Julia community join the
>> main repo and increasing our collaboration with them.
>> 
>> Best,
>> Carin
>> 
>>> On Thu, Sep 27, 2018 at 1:37 PM Chiyuan Zhang <pluskid@gmail.com> wrote:
>>> 
>>> +1 for rebase and merge. As a workaround for the aforementioned issue,
>>> maybe we can create a tag for the commit before the merge, so that in
>> case
>>> people want to browse the recent main-repo commits by skipping this big
>>> chunk of rebased commits, there is a pointer to take his or her hand on.
>>> 
>>> Best,
>>> Chiyuan
>>> 
>>>> On Thu, Sep 27, 2018 at 7:34 AM Jason Dai <jason.dai@gmail.com> wrote:
>>>> 
>>>> +1 to rebase and merge to preserve and track the contributions.
>>>> 
>>>> Thanks,
>>>> -Jason
>>>> 
>>>> On Thu, Sep 27, 2018 at 12:27 PM Aaron Markham <
>>> aaron.s.markham@gmail.com>
>>>> wrote:
>>>> 
>>>>> +1 to rebase and merge to retain the efforts of all of the
>>> contributors.
>>>> If
>>>>> there's some git maintenance that can trim it down from 700+ commits
>>> then
>>>>> maybe that's a compromise.
>>>>> 
>>>>>> On Wed, Sep 26, 2018, 21:23 Naveen Swamy <mnnaveen@gmail.com>
wrote:
>>>>>> 
>>>>>> this PR comes from more than 1 individual, if we squash merge we'll
>>> not
>>>>> be
>>>>>> able to attribute the contribution of those individuals.
>>>>>> 
>>>>>> +1 to rebase merge to preserve history
>>>>>> 
>>>>>> On Thu, Sep 27, 2018 at 12:04 AM, Tianqi Chen <
>>>> tqchen@cs.washington.edu>
>>>>>> wrote:
>>>>>> 
>>>>>>> One of the main reason for a rebase merge is that it preserves
>> the
>>>>> commit
>>>>>>> history of the MXNet.jl package contributors, and given that
the
>>>>> project
>>>>>>> has been evolved since 2015 and has always been a high-quality
>>>> language
>>>>>>> module for MXNet.
>>>>>>> 
>>>>>>> I think we should take an exception here to preserve the commit
>>>> history
>>>>>> of
>>>>>>> each individual contributors to the Julia binding and welcome
>> them
>>> to
>>>>> the
>>>>>>> community.
>>>>>>> 
>>>>>>> Tianqi
>>>>>>> 
>>>>>>> On Wed, Sep 26, 2018 at 8:55 PM Tianqi Chen <
>>>> tqchen@cs.washington.edu>
>>>>>>> wrote:
>>>>>>> 
>>>>>>>> In this particular case, I would suggest rebase and merge.
>>>>>>>> 
>>>>>>>> The main reasoning is that the commit log of the Julia binding
>> is
>>>> not
>>>>>>>> simple WIP commits, every commit there has been done through
>>>>> testcases
>>>>>>> and
>>>>>>>> it is important for us to respect the developer of the effort.
>> It
>>>> is
>>>>>> also
>>>>>>>> good to trace back the history of the commits more easily.
>>>>>>>> 
>>>>>>>> Tianqi
>>>>>>>> 
>>>>>>>> 
>>>>>>>> Tianqi
>>>>>>>> 
>>>>>>>> On Wed, Sep 26, 2018 at 5:34 PM Carin Meier <
>>> carinmeier@gmail.com>
>>>>>>> wrote:
>>>>>>>> 
>>>>>>>>> Chiyuan,
>>>>>>>>> 
>>>>>>>>> Thanks for the prompt to find some clarity of the pros
and
>> cons
>>> of
>>>>>>> each. I
>>>>>>>>> think that will help drive us to the right decision.
I think
>>> some
>>>> of
>>>>>>> those
>>>>>>>>> reasons are the ones you listed. I will take a stab below
at
>>>>> outlining
>>>>>>>>> what
>>>>>>>>> I see. Feel free to chime in if I missed any.
>>>>>>>>> 
>>>>>>>>> *Squash and Merge*
>>>>>>>>>  *Pros* - It is the project standard
>>>>>>>>>          - It will provide one commit for the feature
and
>>> lessen
>>>>> the
>>>>>>> need
>>>>>>>>> for 700+ commits rebased on top of master.
>>>>>>>>>         - It is easier for a user to do git log to browse
>>> commits
>>>>> and
>>>>>>> see
>>>>>>>>> what was features were added.
>>>>>>>>>  *Cons* - I don't know how github would handle squashing
all
>>>> those
>>>>>>> commit
>>>>>>>>> messages into one. Will it be too much?
>>>>>>>>>            - You lose the granularity of the features
>>> individual
>>>>>>> commits
>>>>>>>>> 
>>>>>>>>> *Rebase and Merge*
>>>>>>>>> * Pros *- You don't have a huge commit message with one
>> commit
>>>>>>>>>          -  You do have the granularity of the individual
>>>> features
>>>>> of
>>>>>>> the
>>>>>>>>> commit
>>>>>>>>> * Cons *- It is not the project standard
>>>>>>>>>           - You have 700+ commits on top of master that
might
>>> be
>>>>>> harder
>>>>>>>>> to
>>>>>>>>> see the ones that went in right before. (like someone
browsing
>>>>>> commits)
>>>>>>>>> 
>>>>>>>>> On Wed, Sep 26, 2018 at 8:12 PM Chiyuan Zhang <
>>> pluskid@gmail.com>
>>>>>>> wrote:
>>>>>>>>> 
>>>>>>>>>> Hi Carin,
>>>>>>>>>> 
>>>>>>>>>> Can you clarify the pros and cons of the two approaches?
Is
>>> the
>>>>> main
>>>>>>>>>> concern here about logistics (e.g. preserving the
history of
>>> the
>>>>>>>>> original
>>>>>>>>>> repo and developments) or technical issue (e.g. using
squash
>>>> might
>>>>>> end
>>>>>>>>> up
>>>>>>>>>> with a huuuuge commit message that might be difficult
or
>> hard
>>> to
>>>>>>>>> handle)?
>>>>>>>>>> 
>>>>>>>>>> I think it might not be very likely that someone
is going to
>>>>> cherry
>>>>>>> pick
>>>>>>>>>> revert some of the commits. But preserving the commit
>> history
>>> is
>>>>>> still
>>>>>>>>>> useful in case one need to trace the change or bisect
for
>> some
>>>>>>>>> regression
>>>>>>>>>> bugs, etc.
>>>>>>>>>> 
>>>>>>>>>> Just to provide some context: the PR actually contains
700+
>>>>> commits,
>>>>>>>>> and it
>>>>>>>>>> dates back to 2015. The development of the Julia
binding
>>> started
>>>>> in
>>>>>>> the
>>>>>>>>>> early stage of MXNet. We started with a separate
repo due to
>>> the
>>>>>>>>>> requirement of the package system of julia.
>>>>>>>>>> 
>>>>>>>>>> Best,
>>>>>>>>>> Chiyuan
>>>>>>>>>> 
>>>>>>>>>> On Wed, Sep 26, 2018 at 3:41 PM Carin Meier <
>>>> carinmeier@gmail.com
>>>>>> 
>>>>>>>>> wrote:
>>>>>>>>>> 
>>>>>>>>>>> The Import Julia binding PR ,(
>>>>>>>>>>> https://github.com/apache/incubator-mxnet/pull/10149),
is
>>>>> getting
>>>>>>>>> very
>>>>>>>>>>> close to being merged. Because of the large number
of
>>> commits
>>>>>> there
>>>>>>>>> was a
>>>>>>>>>>> suggestion not to use the usual "Squash and Merge".
 The
>>> only
>>>>>> option
>>>>>>>>>> would
>>>>>>>>>>> be "Rebase and Merge" since merging with a merge
commit is
>>> not
>>>>>>> enabled
>>>>>>>>>> for
>>>>>>>>>>> the project.
>>>>>>>>>>> 
>>>>>>>>>>> *Squash and Merge* - The commits from this branch
will be
>>>>> combined
>>>>>>>>> into
>>>>>>>>>> one
>>>>>>>>>>> commit in the base branch (With all the commit
messages
>>>>> combined)
>>>>>>>>>>> 
>>>>>>>>>>> *Rebase and Merge* - The commits from this branch
will be
>>>>> rebased
>>>>>>> and
>>>>>>>>>> added
>>>>>>>>>>> to the base branch
>>>>>>>>>>> 
>>>>>>>>>>> The PR is over 250+ commits (Github won't show
all of
>> them)
>>>>>>>>>>> 
>>>>>>>>>>> Thoughts about how we should handle the merge?
>>>>>>>>>>> 
>>>>>>>>>>> Thanks,
>>>>>>>>>>> Carin
>>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>> 
>>>>>>>> 
>>>>>>> 
>>>>>> 
>>>>> 
>>>> 
>>> 
>> 

Mime
View raw message