mxnet-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Carin Meier <carinme...@gmail.com>
Subject Re: Which merge option to use on the Import Julia binding PR?
Date Thu, 27 Sep 2018 22:10:07 GMT
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
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message