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 Fri, 05 Oct 2018 00:46:57 GMT
Micheal,

Thanks. You were right! I could merge.

The PR shows up now as merged
https://github.com/apache/incubator-mxnet/pull/10149
My merge commit is here
https://github.com/apache/incubator-mxnet/commits/master

Thanks again for the help.

- Carin



On Thu, Oct 4, 2018 at 8:09 PM Michael Wall <mjwall@gmail.com> wrote:

> I would try the merge locally and then inspect the result closely to make
> sure it looks like what you want.  If it looks good, you could try pushing
> to master.  If you can't push, then we know but I "think" protected just
> means you can't force push in this case based on
> https://issues.apache.org/jira/browse/INFRA-15233 which links to
> https://home.apache.org/~pono/mxnet.png.  Maybe I have only tried that
> with
> repo that own though.
>
> I did find at least one ticket where a team asked for merge commits to be
> enabled, https://issues.apache.org/jira/browse/INFRA-16690.  But I think
> they intend for it to stay that way.  Is that what the community would want
> for the MXNet repo?  Or would you want to enable it for this and disable it
> again?
>
>
> On Thu, Oct 4, 2018 at 7:29 PM Carin Meier <carinmeier@gmail.com> wrote:
>
> > Micheal,
> >
> > Thanks for catching up and helping us with this.
> > I do see the "view command line instructions". I just assumed that master
> > was a protected branch and I would not be able to push to it.
> > Honestly, I'm a bit scared if it isn't :)
> >
> > What do you suggest? Should I try to merge and push to master?
> >
> > On Thu, Oct 4, 2018 at 7:19 PM Michael Wall <mjwall@gmail.com> wrote:
> >
> > > Just now looking at this.  The button is disabled for merge commit as
> you
> > > have mentioned.  Before I go to INFRA, is the command line an option?
> Do
> > > you see "or view command line instructions" beside the green squash and
> > > merge button?
> > >
> > > On Thu, Oct 4, 2018 at 9:09 AM Carin Meier <carinmeier@gmail.com>
> wrote:
> > >
> > > > Thank you Mike!
> > > >
> > > > On Thu, Oct 4, 2018 at 8:54 AM Michael Wall <mjwall@apache.org>
> wrote:
> > > >
> > > > > Hi Carin,
> > > > >
> > > > > I will take a look at this tonight.  I am not tracking everything,
> > so I
> > > > > want to go back and make sure I understand what is being asked.
> > Then I
> > > > am
> > > > > happy to submit an INFRA ticket.
> > > > >
> > > > > Mike
> > > > >
> > > > > On Thu, Oct 4, 2018 at 8:36 AM Carin Meier <carinmeier@gmail.com>
> > > wrote:
> > > > >
> > > > > > I just found out that since we are a podling, we should route all
> > our
> > > > > Infra
> > > > > > tickets through one of our mentors and link the dev list
> discussion
> > > in
> > > > > > JIRA.
> > > > > >
> > > > > > Is there a mentor that is willing to help us navigate this
> process
> > to
> > > > get
> > > > > > the PR merged?
> > > > > >
> > > > > > Thanks,
> > > > > > Carin
> > > > > >
> > > > > > On Tue, Oct 2, 2018 at 8:42 AM Carin Meier <carinmeier@gmail.com
> >
> > > > wrote:
> > > > > >
> > > > > > > Marco - Thanks for the "dry run" idea. It will give everyone a
> > > clear
> > > > > idea
> > > > > > > of the process and what the expected results will look like.
> > > > > > >
> > > > > > > - I took my fork of the repo and synced my master branch.
> > > > > > > - @iblis17 made a copy of the branch of the Julia import PR and
> > > > > submitted
> > > > > > > it to my repo
> > > > > > > - I merged it with the "Merge" option through the web
> interface.
> > > > > > >
> > > > > > > Here is a gif of the process of merging:
> > > > > > > http://g.recordit.co/DzBcFtnjmV.gif
> > > > > > > Here is the result of the repo:
> > > > > > > https://github.com/gigasquid/incubator-mxnet
> > > > > > >
> > > > > > > Please everyone take a look and validate that this looks ok.
> > > > > > >
> > > > > > > If there are no objections, Marco - could you please take the
> > lead
> > > in
> > > > > > > requesting the actions from INFRA?
> > > > > > >
> > > > > > > It will be great to *finally* get this PR in  :)
> > > > > > >
> > > > > > > Thanks,
> > > > > > > Carin
> > > > > > >
> > > > > > > <
> > > https://github.com/gigasquid/incubator-mxnet/commits?author=iblis17
> > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > On Sat, Sep 29, 2018 at 10:02 PM Chiyuan Zhang <
> > pluskid@gmail.com>
> > > > > > wrote:
> > > > > > >
> > > > > > >> Sorry, here is the image: https://imgur.com/V5wd2XB
> > > > > > >>
> > > > > > >> And here is the github document on the 3 different merge
> options
> > > for
> > > > > the
> > > > > > >> web UI button:
> > > > > > >> https://help.github.com/articles/about-pull-request-merges/
> > > > > > >>
> > > > > > >> On Sat, Sep 29, 2018 at 6:48 PM Marco de Abreu
> > > > > > >> <marco.g.abreu@googlemail.com.invalid> wrote:
> > > > > > >>
> > > > > > >> > Could you upload the picture somewhere please? HTML is being
> > > > > stripped
> > > > > > >> out
> > > > > > >> > on email lists.
> > > > > > >> >
> > > > > > >> > Chiyuan Zhang <pluskid@gmail.com> schrieb am So., 30. Sep.
> > > 2018,
> > > > > > 03:44:
> > > > > > >> >
> > > > > > >> > > There is an option in the repo settings menu to disable or
> > > > enable
> > > > > > >> > > merge-commit for PR, see a screenshot below (from a
> > different
> > > > > github
> > > > > > >> > > project):
> > > > > > >> > >
> > > > > > >> > > [image: image.png]
> > > > > > >> > >
> > > > > > >> > > My guess is that this is disabled for the reason to avoid
> > > > creating
> > > > > > >> > > non-linear history for standard PRs (as oppose to
> technical
> > > > > > problem).
> > > > > > >> But
> > > > > > >> > > this is only my guess, it would be great if someone could
> > > > confirm.
> > > > > > >> > >
> > > > > > >> > > Best,
> > > > > > >> > > Chiyuan
> > > > > > >> > >
> > > > > > >> > > On Sat, Sep 29, 2018 at 3:50 AM Carin Meier <
> > > > carinmeier@gmail.com
> > > > > >
> > > > > > >> > wrote:
> > > > > > >> > >
> > > > > > >> > >> I believe so, but if someone wants to confirm it would be
> > > > great.
> > > > > > >> > >> Unfortunately, I just came down with a cold/flu so I will
> > be
> > > > out
> > > > > of
> > > > > > >> > >> communication for a bit
> > > > > > >> > >>
> > > > > > >> > >> On Fri, Sep 28, 2018 at 9:51 PM Marco de Abreu
> > > > > > >> > >> <marco.g.abreu@googlemail.com.invalid> wrote:
> > > > > > >> > >>
> > > > > > >> > >> > Are we sure that this is due to lacking permissions and
> > not
> > > > > > >> because of
> > > > > > >> > >> some
> > > > > > >> > >> > technical limitation? If we are certain, we can ask out
> > > > mentors
> > > > > > to
> > > > > > >> > >> create a
> > > > > > >> > >> > ticket with Apache Infra to make that switch.
> > > > > > >> > >> >
> > > > > > >> > >> > -Marco
> > > > > > >> > >> >
> > > > > > >> > >> > Carin Meier <carinmeier@gmail.com> schrieb am Sa., 29.
> > > Sep.
> > > > > > 2018,
> > > > > > >> > >> 01:17:
> > > > > > >> > >> >
> > > > > > >> > >> > > I made a test regular merge commit into a copy of
> > master.
> > > > It
> > > > > > >> seemed
> > > > > > >> > >> to go
> > > > > > >> > >> > > fine. Here is a listing of what it will look like for
> > > > > everyone.
> > > > > > >> > >> > >
> > > > > > >> > >> > >
> > > > > > >> > >> >
> > > > > > >> > >>
> > > > > > >> >
> > > > > > >>
> > > > > >
> > > > >
> > > >
> > >
> >
> https://github.com/apache/incubator-mxnet/commits/test-merge-julia-import
> > > > > > >> > >> > >
> > > > > > >> > >> > > Although, I would be happy to push the merge button.
> I
> > > > think
> > > > > > the
> > > > > > >> > most
> > > > > > >> > >> > > important thing is to get the PR merged, so whatever
> > way
> > > is
> > > > > the
> > > > > > >> best
> > > > > > >> > >> to
> > > > > > >> > >> > > make that happen, let's do it.
> > > > > > >> > >> > >
> > > > > > >> > >> > > So - Does the regular merge seem like a good option?
> > > > > > >> > >> > > If so, what is the best way to make that happen?
> > > > > > >> > >> > >
> > > > > > >> > >> > >
> > > > > > >> > >> > > On Fri, Sep 28, 2018 at 6:05 PM Chiyuan Zhang <
> > > > > > pluskid@gmail.com
> > > > > > >> >
> > > > > > >> > >> wrote:
> > > > > > >> > >> > >
> > > > > > >> > >> > > > Agreed with Pedro. Maybe the merge-commit option
> from
> > > the
> > > > > > >> github
> > > > > > >> > >> > > interface
> > > > > > >> > >> > > > was disabled for a reason. But as Pedro said, maybe
> > it
> > > is
> > > > > > good
> > > > > > >> to
> > > > > > >> > >> > > > temporarily enable it for this PR and merge using
> > that.
> > > > > > >> > >> > > >
> > > > > > >> > >> > > >
> > > > > > >> > >> > > >    - It should be technically easier than rebasing
> > due
> > > to
> > > > > the
> > > > > > >> > >> > > >    git-subtree-import issue we are currently having
> > > > > > >> > >> > > >    - It also avoid stacking a huge commit history
> on
> > > > *top*
> > > > > of
> > > > > > >> > >> current
> > > > > > >> > >> > > >    history
> > > > > > >> > >> > > >    - The downside is probably the history of the
> > > project
> > > > is
> > > > > > not
> > > > > > >> > >> linear
> > > > > > >> > >> > > >    anymore, but I think this is actually what we
> > would
> > > > like
> > > > > > to
> > > > > > >> > have
> > > > > > >> > >> for
> > > > > > >> > >> > > > this
> > > > > > >> > >> > > >    particular case, because the contents of the
> main
> > > repo
> > > > > and
> > > > > > >> the
> > > > > > >> > >> julia
> > > > > > >> > >> > > > branch
> > > > > > >> > >> > > >    actually does not overlap. So it makes sense to
> > have
> > > > two
> > > > > > >> tails
> > > > > > >> > >> with
> > > > > > >> > >> > > > their
> > > > > > >> > >> > > >    own history.
> > > > > > >> > >> > > >
> > > > > > >> > >> > > > Carin: I guess if someone with admin permission on
> > the
> > > > > github
> > > > > > >> > could
> > > > > > >> > >> > > > temporarily enable the merge-commit option, then
> > > pushing
> > > > > the
> > > > > > >> > button
> > > > > > >> > >> on
> > > > > > >> > >> > > the
> > > > > > >> > >> > > > web might simply work.
> > > > > > >> > >> > > >
> > > > > > >> > >> > > > Best,
> > > > > > >> > >> > > > Chiyuan
> > > > > > >> > >> > > >
> > > > > > >> > >> > > > On Fri, Sep 28, 2018 at 2:53 PM Carin Meier <
> > > > > > >> carinmeier@gmail.com
> > > > > > >> > >
> > > > > > >> > >> > > wrote:
> > > > > > >> > >> > > >
> > > > > > >> > >> > > > > Pedro - Maybe a merge commit is a better answer
> in
> > > this
> > > > > > >> case. I
> > > > > > >> > >> > > > originally
> > > > > > >> > >> > > > > ruled it out since it wasn't an option in the
> > github
> > > > web
> > > > > > >> > >> interface,
> > > > > > >> > >> > but
> > > > > > >> > >> > > > > since this looks like it is going to have to be
> > done
> > > > > > outside
> > > > > > >> it
> > > > > > >> > >> > because
> > > > > > >> > >> > > > of
> > > > > > >> > >> > > > > the subtrees anyway, it might be a better fit.
> > > > > > >> > >> > > > >
> > > > > > >> > >> > > > > On Fri, Sep 28, 2018 at 5:07 PM Carin Meier <
> > > > > > >> > carinmeier@gmail.com
> > > > > > >> > >> >
> > > > > > >> > >> > > > wrote:
> > > > > > >> > >> > > > >
> > > > > > >> > >> > > > > > We are actually running into troubles with
> using
> > > the
> > > > > > >> subtree
> > > > > > >> > and
> > > > > > >> > >> > the
> > > > > > >> > >> > > > > > rebase. Since it looks like this is not going
> to
> > > be a
> > > > > > >> simple,
> > > > > > >> > >> > "click
> > > > > > >> > >> > > > the
> > > > > > >> > >> > > > > > button" through the web page merge, I rather
> hand
> > > > this
> > > > > > task
> > > > > > >> > off
> > > > > > >> > >> to
> > > > > > >> > >> > > > > someone
> > > > > > >> > >> > > > > > with more context in making sure it gets in
> there
> > > > > > >> correctly.
> > > > > > >> > >> > > > > >
> > > > > > >> > >> > > > > > Chiyuan or any others, would you be willing to
> > take
> > > > > this
> > > > > > >> over?
> > > > > > >> > >> > > > > >
> > > > > > >> > >> > > > > > Thanks,
> > > > > > >> > >> > > > > > Carin
> > > > > > >> > >> > > > > >
> > > > > > >> > >> > > > > > On Fri, Sep 28, 2018 at 5:00 PM Naveen Swamy <
> > > > > > >> > >> mnnaveen@gmail.com>
> > > > > > >> > >> > > > wrote:
> > > > > > >> > >> > > > > >
> > > > > > >> > >> > > > > >> 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
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message