On Wed, Oct 22, 2014 at 1:32 PM, Galder Zamarreño <[email protected]> wrote:
> Guys, Jason from Wildfly provided some interesting information a while > back on the benefits of “merge” approach vs cherry-pick. To paraphrase: > > > I used to be anti-merge because I thought it made things harder for > users to grok. That was back when git wasn’t mainstream though. > > > > Now that everyone uses git I think its a good thing. There are some > really nice benefits to it: > > > > 1. The original history from the author is preserved > TBH most of the time I don't care about my history, I always have stupid commit messages until I squash my commits and "prettify" the commit messages. > > 2. The author does not have to toss their branch to avoid a conflict > introduced by a pull after their PR is merged > I think this can only happen if the branch had conflicts and the committer resolved them, but we require authors to rebase so I don't think this has been a problem for us. > > 3. Changes introduced by conflict resolution are kept separate from the > authors. So you know if the problem was caused by the merge or the change > > itself > 4. The person who merged the change is recorded in the git history, so > you have an audit record of who allowed the change in if you have multiple > mergers > Git records the committer as well. You just have to do a "git rebase -f master" locally to make sure the committer field is updated before you push into upstream. > > 5. PRs sometimes include multiple commits, and a merge commit allows you > to see which commits encompass the overall change > > 6. Due to 5, bisecting is quicker > +1, bisecting is cool, even though I've never been able to use it on Infinispan :) > > 7. It’s easier to revert a merge commit > > 8. Github PRs automatically close when you perform a merge > > 9. You can use the big green button with automated CI > > > > There are however some drawbacks: > > > > 1. If you revert a merge, you need to create a new merge to bring it > back. This can be a little confusing if you do it wrong > > 2. You have to know how to interact with merge commits in the tools > (e.g. revert requires -m 1) > 3. git log, for whatever reason defaults to date ordering instead of > topographical ordering, which can look confusing since it doesn’t represent > when > changes were actually merged. Thats solvable using git log > —topo-order > > > Merging merge commits is of course nasty, but you don’t have to allow > it. You can just require that authors rebase their history when they need > it to > be more current vs a git pull. Merging then follows a nice clear > one level nesting.” > > The key thing IMO is: Avoiding merge commits but making sure that everyone > rebases their changes :) > > So, +1 to Tristan’s suggestion but making sure we avoid merge commits! > Sorry, you've lost me here :) Doesn't every merge have a merge commit? I have seen recommendations to use the GitHub Merge button but avoid force the PR authors to rebase anyway elsewhere, but I'm not sure how we could enforce that. I don't think there is any cue in the GitHub UI that the PR could/should be rebased. Maybe we could add a git hook to do that check, and force the integrator to rebase locally if the PR is not yet rebased? > > On 20 Oct 2014, at 18:55, Emmanuel Bernard <[email protected]> wrote: > > > So you review locally and potentially run locally and then you switch > from your terminal console or IDE to wherever the button is in your 350 > opened tabs because it’s faster than git push upstream master. I am having > a hard time to see the convenience unless you do browser only reviews. > > > > > > On 20 Oct 2014, at 18:40, Tristan Tarrant <[email protected]> wrote: > > > >> Sure, you still want to review it in your IDE, and maybe run local > >> tests, but ultimately merging via the GitHub UI. > >> > >> Tristan > >> > >> On 20/10/14 18:37, Emmanuel Bernard wrote: > >>> rebase is a oneliner op per branch you want to reapply whereas cherry > >>> picking requires to manually select the commits you want. Underneath > >>> in git guts it probably does the same. > >>> > >>> I have to admit I barely had the occasion to want to click the GitHub > >>> UI button as except for simple documentation, reviewing code almost > >>> always require to fetch the branch and look at it in an IDE of sort > >>> for proper review. The documentation bit is actually even requiring > >>> local run since Markdown / Asciidoc and all tend to silently fail a > >>> syntax mistake. > >>> > >>> On 20 Oct 2014, at 18:28, Mircea Markus <[email protected] > >>> <mailto:[email protected]>> wrote: > >>> > >>>> > >>>> On Oct 20, 2014, at 17:21, Emmanuel Bernard <[email protected] > >>>> <mailto:[email protected]>> wrote: > >>>> > >>>>> There is a difference between cherry picking and rebasing when it > >>>>> comes to reapply a work on top of a branch. > >>>> > >>>> What is the difference? :-) > >>>> > >>>>> Do you dislike both equally compared to a merge (aka railroad nexus > >>>>> git history approach)? > >>>> > >>>> Using github's "merge" button is pretty convenient imo, even though > >>>> the history is not as nice as with a rebase (or cherry-pick, I miss > >>>> the difference for now ) > >>>> > >>>>> > >>>>> > >>>>> On 20 Oct 2014, at 16:47, Tristan Tarrant <[email protected] > >>>>> <mailto:[email protected]>> wrote: > >>>>> > >>>>>> Hi guys, > >>>>>> > >>>>>> with the imminent release of 7.0.0.CR2 we are reaching the end of > this > >>>>>> release cycle. There have been a ton of improvements (maybe too > many) > >>>>>> and a lot of time has passed since the previous version (maybe to > >>>>>> much). > >>>>>> Following up on my previous e-mail about future plans, here's a > >>>>>> recap of > >>>>>> a plan which I believe will allow us to move at a much quicker pace: > >>>>>> > >>>>>> For the next minor releases I would like to suggest the following > >>>>>> strategy: > >>>>>> - use a 3 month timebox where we strive to maintain master in an > >>>>>> "always releasable" state > >>>>>> - complex feature work will need to happen onto dedicated feature > >>>>>> branches, using the usual GitHub pull-request workflow > >>>>>> - only when a feature is complete (code, tests, docs, reviewed, > >>>>>> CI-checked) it will be merged back into master > >>>>>> - if a feature is running late it will be postponed to the > >>>>>> following minor release so as not to hinder other development > >>>>>> > >>>>>> I am also going to suggest dropping the cherry-picking approach and > >>>>>> going with git merge. In order to achieve this we need CI to be > >>>>>> always in top form with 0 failures in master. This will allow > >>>>>> merging a PR directly from GitHub's interface. We obviously need to > >>>>>> trust our tools and our existing code base. > >>>>>> > >>>>>> This is the plan for 7.1.0: > >>>>>> > >>>>>> 13 November 7.1.0.Alpha1 > >>>>>> 18 December 7.1.0.Beta1 > >>>>>> 15 January 7.1.0.CR1 > >>>>>> 30 January 7.1.0.Final > >>>>>> > >>>>>> > >>>>>> Tristan > >>>>>> > >>>>>> _______________________________________________ > >>>>>> infinispan-dev mailing list > >>>>>> [email protected] <mailto: > [email protected]> > >>>>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev > >>>>> > >>>>> > >>>>> _______________________________________________ > >>>>> infinispan-dev mailing list > >>>>> [email protected] <mailto: > [email protected]> > >>>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev > >>>> > >>>> Cheers, > >>>> -- > >>>> Mircea Markus > >>>> Infinispan lead (www.infinispan.org <http://www.infinispan.org/>) > >>>> > >>>> > >>>> > >>>> > >>>> > >>>> _______________________________________________ > >>>> infinispan-dev mailing list > >>>> [email protected] <mailto:[email protected] > > > >>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev > >>> > >>> > >>> > >>> _______________________________________________ > >>> infinispan-dev mailing list > >>> [email protected] > >>> https://lists.jboss.org/mailman/listinfo/infinispan-dev > >> > >> _______________________________________________ > >> infinispan-dev mailing list > >> [email protected] > >> https://lists.jboss.org/mailman/listinfo/infinispan-dev > > > > > > _______________________________________________ > > infinispan-dev mailing list > > [email protected] > > https://lists.jboss.org/mailman/listinfo/infinispan-dev > > > -- > Galder Zamarreño > [email protected] > twitter.com/galderz > > > _______________________________________________ > infinispan-dev mailing list > [email protected] > https://lists.jboss.org/mailman/listinfo/infinispan-dev >
_______________________________________________ infinispan-dev mailing list [email protected] https://lists.jboss.org/mailman/listinfo/infinispan-dev
