I assume the conclusion is: we keep our current git practice. Right? Based on this practice, is there any easy way for people like Jianfeng to make their merge into their branch simpler? I think Young-Seok is doing experiencing something similar to merge the master into his one-year-behind geo branch.
Chen On Fri, Sep 18, 2015 at 1:04 AM, Chris "Ceej" Hillery <[email protected]> wrote: > I'm pulling this conversation into a separate thread from the "Commit > message proposal". > > There are certainly long-running and contentious arguments about whether > commits to master should be merges or squashes/cherry-picks. That ranks > right up there with vi vs. emacs. Before going too far into that, however, > you should be aware of some additional quirks/limitations that Gerrit > brings into the story. > > First, we have Gerrit configured to always cherry-pick changes when > submitting them. We do this for a couple of reasons, but the main one is > that Gerrit gives you a little additional value in this case: It introduces > Reviewed-on: and Reviewed-by: headers into the commit, which provide you > with a link back to the Gerrit review. For instance, > https://github.com/apache/incubator-asterixdb-hyracks/commit/d885779b13c4fd9bd551d306a460df2894fb18cb > - see that there is a hyperlink there back to Gerrit. This is not possible > if you use any merge strategy other than cherry-pick, because only > cherry-picking creates a *new* commit object that Gerrit can decorate. > > Moving on - In Gerrit, it is not possible to review a "branch merge", at > least not in the way you would hope. A review in Gerrit is always of a > *single commit*. If you make a number of commits on your local working > branch and then push that branch to Gerrit's refs/for/master, it will > create one review for each commit. This is occasionally desirable for > larger changes that can reasonably be broken up, but as a default working > method it is usually more trouble than it's worth - especially because > Gerrit won't prevent you from Submitting a subset of the changes or merging > them out of order. This is the main reason that I implemented git-gerrit to > do a squash before uploading a change to Gerrit. > > What if you make changes on a branch, then merge them to your local master > and then propose just the merge commit to Gerrit? That's just one commit > (the merge commit) and therefore one review, right? Well, no - that would > only work if all of your branch commits were already known to Gerrit, say > because they were submitted (one at a time) to a non-master branch. > Otherwise, you'll get N+1 separate reviews on Gerrit, and the merge commit > review won't even make sense anymore. > > It *is* possible to do this "right" with Gerrit if you want to get merge > commits onto the master branch. You need to create a secondary branch, > "unstable" or "testing" or similar, and propose your individual changes > there. We could even create multiple of those branches for individual > feature work if we wanted. Then once that branch is in a state that you > want to merge to master, you can propose a single merge commit change to > Gerrit. The biggest downside to this is that when reviewing a merge commit, > Gerrit does NOT show you any information about the changes being introduced > into the target branch. It only shows you any diffs which were introduced > during conflict resolution of the merge. Also, merge commits cannot be > cherry-picked, so you lose the Reviewed-on: header for the merge itself. > These aren't necessarily fatal limitations - we have a couple teams in > Couchbase using exactly this methodology with success. > > Anyway. The upshot is, handling git history is a mess, and Gerrit makes it > messier. The "always squash branches, always cherry-pick changes" approach > that we currently use for AsterixDB seems to me to be the least available > evil, and it at least results in a clean if abbreviated commit history on > master. It definitely has its downsides as well, though - Jianfeng's > experience is one such problem, and it doesn't have a clean solution. > > Ceej > aka Chris Hillery
