Check-in before code review is an antipattern
jamesmckay.net
jamesmckay.net
I thought that's what the title meant as well.
That's the very first thing that occurred to me, actually.
If true, that seems like Stone Age code workflow. You might as well argue that code should be tested before deployment rather than edited on production directly, or that it's a really neat idea to keep track of who changed what code when. Duh?
The branch-merge default of git is really remarkable, and is baked into the tool and the community.
Plus anybody who advocates the use of git rebase --interactive as a sensible response to code review feedback is clearly insane.
A completely misguided rant at that. The author even admits it: "To be fair, you can adopt this workflow with Subversion, using either task branches or patches..." This is exactly how my team uses Subversion (and used CVS before it; yes, we used this exact "branch per issue, code review before merge" workflow with CVS up through 2013) and we don't find the overhead of branching and merging onerous. Granted, I haven't ever tried contributing to an open source project using SVN, but arguably Git is the more-correct tool for that kind of development anyways.
Why? Prior to a series of changes being pulled into someone else's history you have all the incentive in the world to present the cleanest view of history possible. Moreover, if you can rebase -i on top of master you can turn the merge into a fast-forward rather than an actual merge. A more linear history with fewer merges is much easier to bisect as well as to read, generally.
Au contraire, rebase can break bisect by creating commits that were never actually tested or compiled. And if your bisection algorithm cannot handle merges, then that's a problem with a bad bisection algorithm.
Nor does it make history more readable; it makes history more readable in the same sense that inlining all procedures makes a program more readable (i.e. not really).
Merge commits are an abstraction mechanism. You can view a branch's history as a purely linear history (git log --first-parent) of normal and merge commits, where merge commits represent the merged branches (and where the commit message should summarize the branch changes properly). You can then unfold merge commits and view the branches they represent in such a quasi-linear fashion, too. Repeat as needed.
This can also guide a bisection algorithm (bisect along git log --first-parent, descending down the merge hierarchy as necessary) and is generally more readable if presented properly (that Git doesn't have a tool other than git log --first-parent for this is a different story). Conversely, a fully linearized history is longer and lacks any structure (as I said, it's like manually inlining all procedures in a piece of code).
This is only true if you allow commits that haven't individually passed your CI suite to be pushed. That's insane.
> Nor does it make history more readable; it makes history more readable in the same sense that inlining all procedures makes a program more readable (i.e. not really).
Working at an organization (Google) that for the most part doesn't use branches and where other merges are impossible (every file must have previously been resolved against whatever is at HEAD prior to submitting), I respectfully disagree[0]. A linear history of individually reviewed and validated changes is MUCH easier to deal with than the regularly branched/merged history of my previous gig (which did use git, along with pre-push change review for each change).
> Merge commits are an abstraction mechanism. You can view a branch's history as a purely linear history (git log --first-parent) of normal and merge commits, where merge commits represent the merged branches (and where the commit message should summarize the branch changes properly). You can then unfold merge commits and view the branches they represent in such a quasi-linear fashion, too. Repeat as needed.
They're a leaky abstraction, though. Unless you mutate published history (which I do think is insane) it's inevitable that your nice bundle of merged changes will one day be incomplete: additional changes to make the bundle actually function as intended will be required. These will end up detached from it as some other commit series on HEAD or merged into HEAD. I find it far easier to tie together a series of changes via things like issue tags in the commit than expecting a merge commit to properly bundle things up.
[0]: Mind you, Google also for the most part does not use git -- we do our own thing, and with that history is necessarily linear, but I've found I really do prefer it that way. See: http://www.wired.com/2015/09/google-2-billion-lines-codeand-...
It's also pretty common in open source projects (due to limited CI resources or other reasons). I also note that this is not the only problem: the problem is that these commits never existed in the wild. This can also lead to subtle problems that CI simply can't catch.
> Working at an organization (Google) that for the most part doesn't use branches and where other merges are impossible (every file must have previously been resolved against whatever is at HEAD prior to submitting), I respectfully disagree[0]. A linear history of individually reviewed and validated changes is MUCH easier to deal with than the regularly branched/merged history of my previous gig (which did use git, along with pre-push change review for each change).
Your problem here is that Git does not have a good way of displaying structured history and just throws the raw DAG or a topologically or chronologically sorted list at the user; lack of named branches means that you're largely looking at an anonymous soup of objects with no tools to filter them. Contrast with Bazaar's hierarchical logs (augmented with branch nicknames), for example, if you want to make a serious argument.
> They're a leaky abstraction, though. Unless you mutate published history (which I do think is insane)
Published mutable history is not per se insane. Please familiarize yourself with Darcs/Pijul/Mercurial's changeset evolution. Note that I am not saying this is necessarily a good idea, it's just not per se insane. Not all projects have the same requirements/workflow/number of contributors/etc.
> it's inevitable that your nice bundle of merged changes will one day be incomplete: additional changes to make the bundle actually function as intended will be required. These will end up detached from it as some other commit series on HEAD or merged into HEAD.
Note that when I say that this is an abstraction mechanism, it's not the only abstraction mechanism. This, again, is a Git problem [1], not a VCS problem. This is, for example, where named branches come into play; see in particular Fossil's tagging mechanism (which has little to do with Git-style tags) or Veracity's stamps.
[1] Git is a solid workhorse for a DVCS, but it is a very basic version control system, sort of the lowest common denominator in the DVCS world. This makes it a poor basis for arguments as to what can be done in a VCS.
* I can't spell
* fix tests, too, lol
* fixed for real
* fixed
* minor touchup
* Implement feature foobar from issue #123
In a way, Perforce is a cute middle ground. It has "pending changelists" that ReviewBoard or Phabricator can slurp up and present to reviewers. You can easily give another person your changes by "shelving" them. Mutable history is only really a "feature" in tiny projects; in massive ones, immutability simplifies life greatly and restores peace of mind. In many ways, the way Perforce handles conflicts is nicer: it doesn't pollute the actual files with conflict markers, and has an option to resolve conflicts in a nice 3-pane UI. Although it does get merges wrong every now and then.
Non-linear history is considered an "anti-feature" by many, and the LLVM community runs a primary SVN server; users submit via git-svn.
Having cheap and lightweight branching is what leads to merge headaches. In fact, (from one Tech Talk), I believe that Google runs a single branch to evade merge problems altogether.
I think git's greatest asset is the excellent local development experience. You can muck around as much as you like, making local branches/commits as checkpoints, and squash them in to submit a final patch. The command-line UI is fantastic, and everything is snappy (provided you have a small checkout). I think having a single linear "trunk" on the server-side is a good idea.
> Having cheap and lightweight branching is what leads to merge headaches. In fact, (from one Tech Talk), I believe that Google runs a single branch to evade merge problems altogether.
Running a single branch also gets rid of the nonlinear history problem. We do it where I work and it works great. Every developer submits PRs from feature branches on their forked copy of the main repo, which get merged in after review and testing. At any time, we can roll back to the previously deployed version of prod if we have to.
Mutable history is also nice for these forked repos, since it allows us to squash commits and eliminate any wrong turns we take in the development process, leaving the main repo's history cleaner.
And I wouldn't necessarily characterize SVN branching as "clunky, user-unfriendly and error-prone". It might be slightly worse than Git, but with modern tools like Eclipse there's hardly a difference. I've never found SVN branching difficult at all.
Once it was approved, you committed. A hook in the svn server would check that you had a link to the reviewboard entry, the entry was approved, and that your changes matched the diff that was approved.
> Subversion makes branching and merging so clunky, user-unfriendly and error-prone that it simply isn’t practical
Is this really a technical problem, or is it more cultural? I'm not very familiar with Git... is there really something there that makes this fundamentally easier?
Branches are also very easy to move around the revision history. If I created a branch 3 months ago, and I wanted to go back and pull that back onto mainline, I can easy 'rebase' those changes as if I created the branch off of the most recent commit. Again, it's a single command that, barring merge conflicts, just works.
Branches can also be pushed and pulled from one machine to another. Got started working on a feature, but can't finish it in this sprint? Have a coworker pull that branch to their box and pick it up. Made an experimental change? Push that branch back to the server so others can take a look.
Git branching absolutely changed the way I write code. I create branches all the time, because they are so cheap I can use them to mess around with small changes without worrying about messing up mainline or my current feature branches.
Regardless, branching and merging in git were designed to be straightforward from the outset. Due to the nature of git's commit graph it is straightforward to tell (with appropriate tools, and this is somewhere that git suffered initially but has improved dramatically as its popularity has waxed) where in history a branch was created and which commits are being merged.
If you want a branch and merge workflow where branches are used frequently and merges are the mechanism by which changes arrive in 'trunk' (which is basically how the Linux kernel is maintained with Linus being the final integrator -- he merges branches from what are effectively lieutenants who act as integrators for their respective subsystems) then git makes this workflow very straightforward.
Teams can still use git or subversion or mercurial and have terrible code review processes, just as they can use Slack or IRC and have terrible communication, or use Ruby, C, Haskell, or ECMAScript 2 and have beautiful or nightmarish code.
Takeaways:
1) Technology is a tool, not an end in itself. 2) People are the hard part. Technology can open the door, but that doesn't mean someone will walk through it.
I don't even disagree with the thesis of the article. its just that the choice of language here obscures the point and makes it harder to focus on what the author is actually trying to say.
I remember SUN's application servers had a licensed code inside their transactional core nobody was able to understand nor touch; it just worked as it was supposed to, prepared by a single genius who got it right.
And whether you like it or not, the best companies rise and fall with geniuses. You can't automate them away. You can only accommodate them or they go elsewhere and you are done.
My guess is not many. The 99% usecase in version control revolves around teams that need to enforce standards.