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 <mjw...@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 <carinme...@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 <mjw...@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 <carinme...@gmail.com> > wrote: > > > > > > > Thank you Mike! > > > > > > > > On Thu, Oct 4, 2018 at 8:54 AM Michael Wall <mjw...@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 <carinme...@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 <carinme...@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 < > > plus...@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.ab...@googlemail.com.invalid> wrote: > > > > > > >> > > > > > > >> > Could you upload the picture somewhere please? HTML is being > > > > > stripped > > > > > > >> out > > > > > > >> > on email lists. > > > > > > >> > > > > > > > >> > Chiyuan Zhang <plus...@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 < > > > > carinme...@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.ab...@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 <carinme...@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 < > > > > > > plus...@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 < > > > > > > >> carinme...@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 < > > > > > > >> > carinme...@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 < > > > > > > >> > >> mnnav...@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.li...@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 < > > > > > > >> > >> > > carinme...@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 > > > < > > > > > > >> > >> > > plus...@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....@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.mark...@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 > > < > > > > > > >> > >> > mnnav...@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 < > > > > > > >> > >> > > > > >> >>>> tqc...@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 > > > > < > > > > > > >> > >> > > > > >> >>>> tqc...@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 < > > > > > > >> > >> > > > > >> >>> carinme...@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 > > > > > > < > > > > > > >> > >> > > > > >> >>> plus...@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 < > > > > > > >> > >> > > > > >> >>>> carinme...@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 > > > > > > >> > >> > > > > >> >>>>>>>>>>> > > > > > > >> > >> > > > > >> >>>>>>>>>> > > > > > > >> > >> > > > > >> >>>>>>>>> > > > > > > >> > >> > > > > >> >>>>>>>> > > > > > > >> > >> > > > > >> >>>>>>> > > > > > > >> > >> > > > > >> >>>>>> > > > > > > >> > >> > > > > >> >>>>> > > > > > > >> > >> > > > > >> >>>> > > > > > > >> > >> > > > > >> >>> > > > > > > >> > >> > > > > >> >> > > > > > > >> > >> > > > > >> > > > > > > >> > >> > > > > > > > > > > > >> > >> > > > > > > > > > > >> > >> > > > > > > > > > >> > >> > > > > > > > > >> > >> > > > > > > > >> > >> > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > >
