mxnet-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chiyuan Zhang <plus...@gmail.com>
Subject Re: Which merge option to use on the Import Julia binding PR?
Date Thu, 27 Sep 2018 17:37:06 GMT
+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