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 > >> > > > > >> >>>>>>>>>>> > >> > > > > >> >>>>>>>>>> > >> > > > > >> >>>>>>>>> > >> > > > > >> >>>>>>>> > >> > > > > >> >>>>>>> > >> > > > > >> >>>>>> > >> > > > > >> >>>>> > >> > > > > >> >>>> > >> > > > > >> >>> > >> > > > > >> >> > >> > > > > >> > >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > >> > > >
