Abandoning Gitflow and GitHub in favour of Gerrit
beepsend.com
beepsend.com
The fact that one commit = one merge is ridiculous. It encourages monolithic commits for no reason other than that the tool demands it. It's unrealistic to ask someone to code review multiple commits per feature, and if you tie in your CI it doesn't make any sense to run your build over and over again for a single feature either.
The patchsets DO allow you to see the history of a code review if you have to make changes, but I'd much rather see that live in my git history rather than in Gerrit's history. It's a dirty solution to have to amend your commits to make changes.
Nowhere does the author mention how painful it is if you finish a feature, are waiting for a code review, but have to start the next feature using the code you just wrote. Maybe I'm missing some magical feature in Gerrit that makes this easy, but if you push multiple dependent commits to Gerrit, and one of the early ones gets merged, all of the later ones now have to be rebased because Gerrit created a merge commit in the middle.
Someone didn't spend time configuring gerrit or configured it wrong for your use cases. You can solve this in one of two ways: you either fast-forward when possible (gerrit can attempt a trivial rebase) or you commit a change set, create your own merge commit, and then your early ones won't get merged until your last commit. Gerrit will wait to merge the early commits (and vice versa if dependant commits are ready) until everything is good, then merge them all in one shot.
I used gerrit heavily for a few years (and still do, passively, with golang and other public google projects) and I miss it dearly. Sure it's not perfect, but I'd take it over the github fork/make pull requests model any day of the week.
It does not encourage monolithic commits. You need developers to understand what they are doing, care about commit history and be disciplined during development. In my experience, developers who just want to hack shit together, cut corners and love squashing commits or throwing away history hate gerrit. Developers who care love it. It's definitely possible and easy to produce Agile software.
Like anything else, you should use the right tool for the right job. If you're not willing to suck it up and learn something new, then it's not going to have a chance to be the right tool.
I think the OP's point was that Garret hides a lot of features that are built into git and then attempts to paper over that fact by re-implemting those features within its own system.
In your first paragraph, you defend Garrit's rebasing procedure and then you implicitly accuse the OP of not being willing to "suck it up and learn something new".
Why should we learn a new tool that makes (arguably poor) attempts at re-implementing the features of a tool that we already know and love?
The discussed tool is not "Garret" or "Garrit", it's Gerrit, which makes me wonder if you used it before commenting.
As far as I know, Gerrit doesn't reimplement git's features. Rebasing is done using git rebase.
Gerrit is a system for code review, just like GitHub's PR, but with a different approach.
At the gerrit-based shop, we kept very good discipline of regular, small checkins. And yes, you can pipeline your checkins too.
I found the gerrit flow also much easier to explain to new engineers. The github PR flow is much more full of sharp edges.
That's one way of looking at it. Another is, it encourages small self contained units that build to become a full fledged feature.
> It's unrealistic to ask someone to code review multiple commits per feature, and if you tie in your CI it doesn't make any sense to run your build over and over again for a single feature either.
Re CI - Every change to the code should be verified by CI. With gerrit, you can choose to execute the tests for every patchset as its uploaded, or only once it's received a number of reviews or been approved.
> The patchsets DO allow you to see the history of a code review if you have to make changes, but I'd much rather see that live in my git history rather than in Gerrit's history.
I actually kind of agree here, if you migrate away from Gerrit, keeping the review history is hard. Though, every revision of every patchset is actually stored in Git, as are a history of the votes (in the form of a `git note` attached to the commit).
> Nowhere does the author mention how painful it is if you finish a feature, are waiting for a code review, but have to start the next feature using the code you just wrote. Maybe I'm missing some magical feature in Gerrit that makes this easy, but if you push multiple dependent commits to Gerrit, and one of the early ones gets merged, all of the later ones now have to be rebased because Gerrit created a merge commit in the middle.
This really sounds like your Gerrit admins just chose some bad config options..
I also had to admin a Gerrit server once and the documentation (at the time at least) for setting up and running a Gerrit server was total shit. It was a painful process to say the least.
Another project, which took many of my old team members, started using Gitlab instead and they loved it. The merge requests made a lot more sense. I'm currently at a new company that uses Gitlab and I have to agree.
Both systems are pretty much suggestive. We could always +1 a code review ourselves if it had to get out that day, but it's best someone else did it and there was a record. Gitlab is a lot more lose. There's no official field for an approval, but you can put in a nice little thumbs up emoji in your comment, and you have the same audit trail.
TL;DR +1 Gitlab (et al.) over Gerrit for sure
A lighter-weight SaaS like https://review.ninja, https://omniref.com, or https://reviewable.io (disclosure: this one's mine) might be a more appropriate solution. Specifically to the article's points, Reviewable has a nice reviews dashboard and will show inter-commit diffs within a PR (whether you're rebasing/amending or not), hiding any files with no changes since you last looked. Its default review completion criterion is that all files have been marked as reviewed by at least one person and there are no unresolved discussions still going on, but you can customize this to your team by writing a snippet of code to run against the review's state, e.g. to implement LGTM approval or even a voting system. Reviewable will also update a status check on the PR so you can enforce review completion before merging if that works best for your situation.
Best of all, because both systems integrate tightly with GitHub, there's no need to learn a new workflow or mess around with new git commands. Gerrit still has its place but I don't think it should be the tool of first resort.
(Edit: added mention of Omniref.)
Someone neatly wrote up the main advantages:
http://cramer.io/2014/05/03/on-pull-requests
Phabricator's issue tracker is also an excellent choice over GitHub's simplistic issue tracker.
Also, Gerrit isn't that hard and I've seen small teams get productive with it within a 1-2 weeks.
No need to reinvent the wheel!
By the way: I live in Europe and I haven't worked for one single company which would allow their developers to host proprietary source code with a third party SaaS provider.
> By the way: I live in Europe and I haven't worked for one single company which would allow their developers to host proprietary source code with a third party SaaS provider.
Fair enough, companies vary widely in their acceptance of SaaS -- though Reviewable has plenty of European customers too. But to clarify, neither Review Ninja nor Reviewable (not sure about Omniref) actually host code themselves: they just access it through GitHub APIs without storing it. You can also deploy Review Ninja (and soon Reviewable) on-premises, though of course that means you're on the hook for administrating the system again.
That's the nice thing about Phabricator, you can switch off all features you don't need, and it integrates with GitHub. You can definitely use it just for code reviews, with users logging in using their GitHub accounts and the repositories being hosted by GitHub.
> You can also deploy Review Ninja (and soon Reviewable) on-premises, though of course that means you're on the hook for administrating the system again.
That would work! Third party SaaS probably includes Github.
One small correction I would like to make to gitflow is that the default branch for developers should be 'master'. If you want to deploy another branch, use a 'production' branch. This means people don't have to manually change branches everytime they clone, that type of repetitive work should be outsourced to a computer ( your deployment scripts ). If you have full access to the repo and you are on github, atleast you can change the default branch ( which for a repo I contribute to, I don't )
Anyone know the need for a seperate 'develop' branch ?
The HEAD of master should always be production-ready code. It is your most-stable branch.
It's necessary to have a less-stable, separate, develop branch where features, bugfixes, etc, that were developed off on their own sandboxed branches can be merged and integrated as part of a development phase.
Even with the most disciplined testing of isolated feature branches, it's not known whether integrating different ones will always work, so it is not even guaranteed that the HEAD of develop will always be stable, let alone production-ready. Even if it is stable, it could not be production-ready for the simple reason that it so far only contains features X and Y, and the next release cannot go out without Z there too, where Z is not merged yet.
As you point out, having master as the most stable branch appears to just be a convention, and it would be perfectly possible to branch a production branch from master and swap the roles at the start of the project too.
I think the convention is like that so that one can immediately build and run the latest production code after cloning the repo, which in my opinion is nice. Besides, with the number of branch switches you do every day as part of normal development, it doesn't seem too horrible to have to do it once, to start working off of develop after you clone the repo - at the end of the day how often do you do that, and how annoying or arduous really is it?
I don't think it's _necessary_ by any means. e.g. We auto-deploy the HEAD of master, assuming tests pass and don't use any special integration branch. If you have some set of features that need to ship together, that is just a feature branch like all others. Multiple people can work on a feature branch and test their integrations there.
> Even if it is stable, it could not be production-ready for the simple reason that it so far only contains features X and Y, and the next release cannot go out without Z there too, where Z is not merged yet.
I think largely your assumptions are based on a slow moving release cycle, not everyone operates that way. We regularly release multiple versions a day in most of our repos.
The idea in gitflow is that you then isolate a snapshot of that integration branch, whatever form or name it takes, as a release branch, and only when that release branch produces a build passing all tests is it then merged to master (and probably tagged).
You mention you auto-deploy from HEAD of master after tests pass - does that mean that code at the HEAD of master may not pass tests, or that you merge code to master only if it has passed tests. If it's the former, then you're not using gitflow, and I was just explaining the use of separate master and develop in gitflow, not implying that this is the one true branching/release strategy. Using something other than gitflow can work equally well. If the latter, then you are using an integration branch - the one you run the tests on before merging to master.
As to the point about release frequency, having a set of features to release in batch was just an example - if you're in any situation where other developers may commit things to the branch that will ultimately be released from between the time you create your feature branch and the time you merge it, then gitflow is useful - be that on a scale of minutes, hours, days or weeks.
...ok, I guess...
> develop branch where features
...what? Why shouldn't I call my branch for integrating code... integration, or something?
> I think the convention is like that so that one can immediately build and run the latest production code after cloning the repo, which in my opinion is nice.
It's not nice; it's wrong. Who is going to checkout a project code? End users? If so, then it makes sense. However, if it's just developers, it's fair to assume that they do the checkout to develop the project, and so making master a production branch gets in their way. A very typical scenario is when a new dev joins the project, hacks happily on branch forked from master and finally submit the PR with fixes which were already done, and probably already done in a different way, so you have both time wasted and merge conflicts. Don't do that! At the very least set your develop branch as the main one on GitHub, but that's still not as idiot-proof as just using master for development and separate branches and/or tags for production releases.
You can :-) gitflow is just a pattern, the names of the branches are an implementation detail. Just using develop here as it's the name used in the parent's question.
> A very typical scenario is when a new dev joins the project, hacks happily on branch forked from master and finally submit the PR with fixes which were already done..
Gitflow like any branching strategy requires that people using it understand it first, to work. Obviously it's always better to craft these things in ways which help people fall into the pit of success, but if you have new devs just cloning a project, branching however they assume is ok and then fixing bugs that others are working on etc, without once asking a question or being instructed in even the simplest way on what to do (saying 'we use gitflow on this project, development branch is called X' in the readme for example), that's a problem that no branching strategy is going to solve!
I started an embedded project a while back using the gitflow style I was used to, but eventually found the meaning of "master" to be not well defined, once we had different release versions and variations. We eventually killed master, and renamed "develop" to "master", and that particular project makes much more sense now.
Also, if you had different variations of builds etc (I assume for different target hardware) could it have been better organised by putting all the common code in a different project, then creating separate projects for each of the variations with that common code as a dependency? Then you could probably manage each of the variation projects with gitflow as usual.
Or another way to put it is develop doesn't even have to compile. I mean, it would be nice if it does, but its OK if it doesn't.
master might not do what the end users think its supposed to do, but at least it compiles.
Or another way to put it is develop is for big revolutions and master is for small evolutions.
git flow releases are supposed to actually do what the end users think its supposed to do. Or it actually does what they've been told to expect.
It provides a natural flow of code, from a feature doesn't work but its worth a share, to it compiles but doesn't implement business required feature #1243 or feature #531, to we think it correctly meets the business needs at least as of one definition of need at one point. Or another way to put it is a separation of development process of develop to master from the business requirements of master to a release branch.
I suppose everyone uses it a little differently.
https://help.github.com/articles/setting-the-default-branch/
"you lost track of the comments in the code and viewing what had actually changed since the last update became really hard" I don't understand why comments in the code would disappear. Line-comments on the code might disappear when lines are changed. We're adding a feature to GitLab where you can acknowledge line comments. And viewing changes is pretty easy as long as you don't rebase.
For release-to-customer products, we have a release branch, eg: release/1.2.3, and as part of our CI build, that number is automatically picked up and used in the product itself as the version number.
For our cloud/SaaS/single-instance stuff, we have a 'staging' and 'production' branch, and typically have 3 environments (dev, staging/test, and production) all of which are continuously deployed. We typically just do fast-forward merges to bring staging and production up-to-date with master.
In both cases, when a developer is working on a new feature or fix, we want them to base off master.. which is the latest fully-integrated build, though it hasn't passed QA. If they worked off a production/release branch, we'd continuously have to deal with conflicts as by definition they're basing their changes on out-of-date code.
A 'develop' branch could work for some of our small stuff, where typically production/release isn't far off from master, but for our main stuff we do a 2-3 month release cycle, so master usually has quite a lot of changes that aren't in release.
The OpenStack community uses Gerrit pretty widely across our various projects. It might help to check out a busy project like Nova (https://review.openstack.org/#/q/project:openstack/nova,n,z) to get a feel for how Gerrit works in practice. Or a less-busy project like Bandit, which I'm involved in (https://review.openstack.org/#/q/project:openstack/bandit,n,...).
You say clunky, but Phabricator's UI makes Gerrit look like a WAP app.
Disclosure: I'm the founder.
However, I will say the demo is a bit odd. It's pretty much impossible to look at the code diff because there are comments everywhere. And the code diff appears to default to not actually showing a diff (the left and right diff bounds are both set to the latest version), which is especially confusing when it shows side-by-side since it's showing the same revision on both sides.
It's really odd that you got a nil default diff range. I can't reproduce it with either anonymous or authenticated access. If you can, could you please open an issue with more details so I can debug? Thanks!
Also, now that you've reset it, all the diff ranges are now defaulting to the widest view instead of defaulting to just the current revision. Given that I can no longer reproduce it, I'm not sure there's any more detail I can add (beyond the fact that I'm using Safari 9.1 on OS X 10.11.4).
It works seamlessly with GitHub, and provides code reviews that end up "sticking" to your code, and documenting its development. Every pull request you create can become a review automatically, or you can pick-and-choose which pull requests to review. And you don't lose reviews or comments when you push to a branch under review.
Also, it lets you dive into the history of a single line of code. For example, here's a line from GitHub's libgit project, annotated with the pull request that created it: https://www.omniref.com/repositories/libgit2/rugged/files/li...
It's on my feature short-list to write the code to make a best-effort to migrate review comments for rebased commits. But this will always be a bit of a heuristic, and error-prone. If the diff changes substantially as part of a rebase, it's really hard (i.e. theoretically impossible) to always know where to move annotations.
Ultimately, all you really know when you're on the receiving end of a force-push is that some commits were orphaned by a new commit. You can detect this and identify the orphaned commits, but knowing where to move the sticky notes on those orphaned commits is challenging.
You don't have to do this with Github. Just clone directly to your local machine. Also, you don't have to use git flow when using Github. I think git flow seems to be the real culprit of the symptoms you have identified.
If you use this model, your local clone isn't "backedup" by github until the pullrequest is merged, correct?
One reason I like my teammembers to have their own local fork, is so they can have github exist as a backup.
Bad assumption. However, branch protection sounds cool. Will investigate it.
This is similar to the open-source maintainer model.
What if you don't have push access?
Everyone else, by default, has read-only access to the entire organizations private repos.
Each product team is allowed to specify how their own teams can access repos.
We also have 3rd party contractors and vendors work on portions, and they don't have push access.
They don't want to use Github for the code review and prefer to use Gerrit? Good for them, I don't either and actually prefer GitLab or BitBucket for git in general. But hey! thanks for the Gerrit introduction.
But gitflow?
I've been using the gitflow way of working for ~5 years (although not always the gitflow extension) and I don't see anything there that clashes with that way of working. You want to work on a feature, perfect: Branch from develop and work away.
After you are done and before you merge with develop again, you push your feature to Gerrit and do the code review/changes. And then merge back to develop.
Really, I don't see the issue between Gerrit and gitflow and still think that gitflow is a very sane way of working in a team, specially if you have people that are not used to work with [distributed] version control systems and you probably wouldn't believe how many developers are out there like that.
The idea of a more in-depth review is intriguing (we all know this is something that can be improved), but _voting_ on a peer's code just seems like a bad idea. Vote too low, people get insulted and you breed discontent and mistrust. Too high and everyone stays happy until code quality drops. People will either use this as an opportunity to diminish other's, show off or suck up. Sad, but that's human nature.
I know in-person code reviews aren't always possible but adding this disconnect just seems like a bad idea. Would love some honest feedback from people who have actually used it in medium-large scale production.
It sounds like it removes the ambiguity of when you leave a few comments on a PR but aren't explicit about whether you think it should be merged or not.
the moment someone creates a new version control system that has most of what Git does but fixes parallel histories, I'll switch. And I don't mean that the way people say "if Bush wins again I'm moving to Canada." I say that as someone who has administered CVS, SVN and Perforce repositories on behalf of my teammates but wants nothing to do with administering Git.
Basically, the moment two or more people try to work on something outside of the trunk line of code (trunk/master/whatever), there is no version of their commit history that will be pretty, and so squashing the branch at the end seems like a good idea, even though it produces objectively worse code over the long run.
To wit: There comes a point where the branch and merge structure of the code N months ago is no longer relevant to my day. However, the contents of 'blame' could contain code from years ago every time I use it. [edit] How it was merged is irrelevant. When it was merged, by whom and with what commit message is what survives into the future. By squashing the who and the message are lost. But we think that's okay because it's a lesser evil than having a bunch of commits in trunk. Which is bullshit.
What's hard to stomach is a bunch of merge commits back and forth and back again, and so I sympathize with people eager to sweep those under the rug. But at the same time I know that one of the most common ways a bug makes it into the code is via a bad merge. Keeping them is more honest even if we don't want to think about all the little human things we do that make our code worse.
I want to tell one little white lie with the code: I want to pretend like Joe and Tim wrote their entire feature after my bug fix and before Steven's, even though they worked on it all week. I want them to be able to commit it as a single transaction but with all the intermediate steps.
When I say 'parallel history' I mean I want Joe to be able to rebase the branch on top of my changes, without having to go apologize to Tim for making his snapshot into mincemeat.
Or, I want us to stop pretending like feature branches fix all of our problems, with no serious consequences. Maybe we should just go back to the roots of Continuous Integration and merge on every commit, and rely on things like feature toggles and test automation to control the reach of our in-progress changes.
I think the real issue at hand here is the one-size-fits-most mentality we maintain. As a maintainer of a FOSS project I want to reserve the right to reject contributions out of hand. If I don't like your code I tell you no and you go away. I will also enjoy getting medium units of work that were a team effort without ever having to coordinate with any of those team members. As a volunteer effort, this scales like nobody's business. But just because it's working great for open source doesn't mean it's the rational answer for commercial code.
In commercial code, all changes can be tracked to either human error or a requirements change, and knowledge of the project often is locked in someone's head because archived public forums aren't the dominant form of communication or negotiation. When, how, and why every line changed matters because every fix I undo while making another one alienates a paying customer. So verifying why the code is the way it is now is fairly important.
And let's be perfectly honest here. If, as your dev lead, I don't like the quality of your contribution, guess what, it's going in anyway. I can push back and make you do it better, but 9 times out of 10 your change is going in. Maybe not today, but soon, unless I'm in the process of getting you fired for incompetence. So being able to drop it on the floor at no cost to me isn't really a useful feature. For Linus that and an insulting email are how he 'fires' bad contributors. As coworkers our relationship would be a little more complex.
Pretending that these constraints fit the exact same development pattern as what works for Linux, NPM, Mocha, or probably even Docker is just nuts. We can share a lot of tools, but we can't use exactly the same development process. And Git bends in one direction but has no give in the other.
Well, don't erase the message when squashing. My policy is that we take the pull request and use the message title and description as the actual message of the squashed commit, which works great.
Having a bunch of commits IS really bad. One idea = one commit is so much better than some bullshit three commits of "refactored" "made mistake" "back to normal" those are just as worthless to keep in the history as recording your typos+backspaces+fixes into the history.
> Basically, the moment two or more people try to work on something outside of the trunk line of code (trunk/master/whatever), there is no version of their commit history that will be pretty, and so squashing the branch at the end seems like a good idea, even though it produces objectively worse code over the long run.
I don't like squashing at all, unless you're squashing commits locally because you fixed a bug in a previous commit that only exists in your local branch. Because in that case, it doesn't make sense to pollute the history with commits that were known to be broken when merged (this causes issues with bisecting).
> I want to tell one little white lie with the code: I want to pretend like Joe and Tim wrote their entire feature after my bug fix and before Steven's, even though they worked on it all week. I want them to be able to commit it as a single transaction but with all the intermediate steps.
> When I say 'parallel history' I mean I want Joe to be able to rebase the branch on top of my changes, without having to go apologize to Tim for making his snapshot into mincemeat.
I had to read your comment several times and I still don't think I understand what you mean by "parallel history". Are you saying that A and B are working on a feature concurrently (but not by merging into a central place)? If they were merging into a central place than that central place can be rebased to get in line with C's bugfix. Then A and B can rebase their local branches with the central branch.
It's also possible to do all of the rebasing locally, then once one pushes the other needs to rebase. But I don't think that works well.
Essentially you need to treat people collaborating on a branch the same way you consider it as collaborating on a project.
But maybe I misunderstood what you meant.
> Or, I want us to stop pretending like feature branches fix all of our problems, with no serious consequences.
Looks like I did misunderstand.
edit: spelling
The author mentions this as being a feature of Gerrit in the post.
- [Gerrit Code Review for GitHub](http://gerrithub.io/)
I don't want a new hosting service, I just want a code review process on top of GitHub.
I have used Gerrit in my previous team and it did not worked so well. Hanging vetos on -2 are not that nice when you have to push feature forward, like instead of blocking it someone else could just fix it, by the time you talked person who put -2 to change it to -1. But maybe with more mature team it would not be a problem.
There's always hurry, but skipping reviews (and often unit testing too) is a sure way to make sure you'll keep busy fire-fighting in the future.
An example of this is my own team, where we currently have a list of "release/*" branches in our GitHub, because "git flow doesn't deal very well with hotfixing a release".
Fundamental misunderstandings.
It should be worth noting that those large open source projects have very, very different needs than a small development team working on a product together. The open source project likely isn't doing weekly releases (which require some sort of manual QA process, in the source article). A large open source project has hundreds of contributors, where reviewer time is scarcer than contributor time (and the pool of people to approve and commit a change is much smaller than the contributor pool).
I think the OP's real problems are that:
- an increased release frequency requires them to do more QA
- their time spent in code review seems to be a function of how often they are "releasing", not how often people are making changes
If the difficulty of making a release increases as you increase your release rate, you might be doing "agile" in a poor way.
We use github at work with a feature branch workflow (as opposed to gitflow). We've adopted a system where pull request comments are addressed through the use of "fixup" commits.
For example, when a pull request is submitted for a feature branch that contains 3 commits, and a comment is made regarding part of the change, the person who submitted the PR will add a commit that addresses the comment with a commit title of:
>> fixup! Title of the commit to update
>>
>> An explanation of what this commit does and why
>> ...
This, incidently, is exactly what git commit --fixup <commit_ref> does.
Then the person responds to the comment saying that it was addressed in <commit_sha1>.
As a reviewer, it makes it easy to see that my comment has been addressed and exactly what change was made to address it (by clicking on the link that github autogenerates from the sha1 in the comment).
Once the review process is complete, the person will run git fetch origin and then git rebase -i --autosquash --keep-empty origin/master to actually reduce the set of commits down to the original clean set of commits. They then run a git diff <original branch head sha1>.. to verify that there are no differences and then they merge the PR using the merge button in the web interface.
This way, you end up merging a clean set of commits for each PR, and it's still relatively easy to keep track of comments and incremental code changes addressing those comments during the PR.
In fact, multiple developers can collaborate using the same branch by pushing up "fixup!" commits. Though they need to make sure that they fetch/merge or pull before they push to avoid unwanted merge commits within the branch.
Github's workflow is easy. You have a repo, you can fork it or create a feature branch, you can add multiple commits, and then open a PR. That makes sense. And the interface is pretty.
In Gerrit, I still do commit-as-you-go, because that's the entire fucking point of Git. If I wanted SVN semantics in my repo, I'd use SVN. Then, we have to squash all the commits (I'm very anti-history-revision, but I know that's an opinion) and push up to a different origin. And god forbid if you want to work from that code point in a new branch while you wait for a review. Oh, and if you want to fix some issues found in review? Yeah, let's edit the history again... Oh, and there's a change id created that causes all kinds of other headaches.
I have tools to manage and make sense of my git history. I absolutely hate things that force me to modify history. It might as well be voodoo magic when stuff goes wrong. It's often easier to blow it all away and start over.
I do like the things it can help you enforce -- a good build and +1/+2 code review etc. But that's not enough to deal with all the little annoyances in gerrit. Especially since it's available in a much better tool -- gitlab.
Gitlab is what comes after github has run its course for your team. It's got the same predictable and useful feel, it integrates great with CI tools, and it allows a similar GHPR-style way of merging. There's also BitBucket Server and other stuff.. but Gitlab has my vote in the strongest way possible.
Mistake #1.
Why are they all rebasing in their PR branches if it obviously makes the PR unreadable?
Not very flexible, I see a lot of churn of invalid pull requests with this design if they aren't allowed to grow into complete features..
Actually, Gerrit really encourages growing a patchset ("pull request") into a complete feature. It allows you update your change over and over, addressing review comments as they come in.
Once done, you have a clean "Add support for use of XYZ by ABC" commit - and not a pile of half baked commits - I cringe when I see things like this: "Add framework for XYZ", "Define Config for XYZ", "Correct typos", "Add tests", "Rework XYZ to be standards compliant", "Correct typos", "Fix tests"
> Actually, Gerrit really encourages growing a patchset ("pull request") into a complete feature. It allows you update your change over and over, addressing review comments as they come in.
> Once done, you have a clean "Add support for use of XYZ by ABC" commit - and not a pile of half baked commits - I cringe when I see things like this: "Add framework for XYZ", "Define Config for XYZ", "Correct typos", "Add tests", "Rework XYZ to be standards compliant", "Correct typos", "Fix tests"
I don't like commits like that either. But almost all real pull requests require more than one commit (so future generations can bisect the repo properly without then needing to bisect patches as well). A nice pull request is something like this:
server: add statistics monitoring framework
server: component a: hook into statistics monitoring
api: expose statistics monitoring
integration: add tests for statistics
Each commit works as intended and does exactly one thing. I tried to do something like this on Gerrit (was contributing to the TWRP recovery) and it was such a pain that I collapsed everything to one commit. That's not how things should be dealt with (I needed to improve the pattern decryption to support N*N patterns and it required a bunch of UI, internals and other changes that all got squashed together).
I also didn't like the fact that anybody could overwrite your PR's commit with their own crap. Why is that a feature?
One of the largest problems I've had getting gerrit adoption on smaller teams is that they can just pop up a private github team easily, whereas finding hosted gerrit solutions that actually make it easy to convert from other source control tooling has been very difficult.
The process might seem more complex initially but think
of it like this: If you add a new member to your team,
they would have to fork the repositories on GitHub, clone
them locally, make the changes, push to their own fork
and then create the pull request
In addition, the new GitHub code review tools address most (if not all) of the stated reasons for this switch. And in my opinion, GitHub's ease-of-use, alongside it being an incredible single point of reference, far outweigh any clunky other tools I've used (like Gerrit).
Most of the time its people not understanding the tech they're trying to use and instead of spending the time to learn it they look for something they already know how to use or they'll pay someone to do the thinking for them which usually overlooks most of the company's context. I know I'm grossly generalizing but that's the trend I've seen so far.
They'll bash on Git for not being enough like SVN, bash on GitHub/GitLab for not having the same workflow of the previous tool. But will happily pay $200 an hour for someone to tell them what to use/do even if they end up making terrible decisions.
I think the case you describe is where consultants can be a net win, for the very reason you mentioned: dev departments at non-tech companies. These companies are usually happy with "its quirky but works" software since it gets the job done and allows them to focus on their core skills. There's nothing wrong in not having programming as your core expertise.
What I was describing is my experience seeing consultants and Agile brought in at companies where software development is their main area of expertise.
Exactly how everyone I know uses git.
OK so there's a new long complicated extremely labor intensive elaborate detailed process. "Surely there had to be a better alternative?" Yes, yes there is. Unfortunately there are many options.
Now the correct answer would have been simplicate and add lightness. Agile manifesto "Individuals and interactions over processes and tools".
Instead, they found an overwhelmingly clunky and complex (their words not mine) tool to automate the complexity, or at least temporarily abstract it away, or at the very least replace one pile of complexity with another (larger?) pile of complexity.
Lets try a less controversial topic. Getting a pencil. All workplaces have different policies for office supplies. Order your own just like you buy your own clothes. Here's your annual office max gift certificate buy whatever you want. Here's a key to a supply closet. But here, we implemented a 15 page process involving teams of employees reviewing and documenting each individual request for a pencil in meetings. That's the bad news. The good news, is we replaced the original 15 page process with a 5 page process by installing an IBM mainframe, DB2, CICS, and writing a simple COBOL app that allows end users with newly installed 3270 terminals to request a pencil, and now we're better off than "ever before". Now most people would have handed out GCs, given out supply closet keys, or just trusted the employees, so you're going to get all kinds of semi-off topic blowback about how the COBOL program should have been the flavor of the month CRUD web JS framework, or instead of DB2 they really should have used nosql on the cloud so they can scale office supply request to all 100 employees not just a small team. But the real problem that everyone is squirming about is they're doin' it all wrong.
Saying that this is somehow more complex than updating a single commit and learning a whole new tool doesn't change that if you want to convince me, you at least need to correctly identify what's going wrong.
Perhaps if the author had specifically called out their perceived failings of the very latest GitHub pull request changes I'd have given the article more time. But unfortunately the justification given for switching was really shallow.
Yesterday I was looking for a way to refresh an old fork with upstream. I'm pretty sure there was a button for this at one point. I looked. Couldn't find it. So instead I had to:
$ cd ~/src
$ mkdir github
$ cd github
$ git clone myfork
$ cd myfork
$ git remote add up upstream
$ git pull up master
$ git push origin master
Github may be pretty-ish, but I tend to avoid them these days. They're expensive, and they remove useful features. Fool me once, shame on you, fool me twice can't get fooled again.
https://gist.github.com/xenophonf/9df09e47a8629bb789ffbb94c7...
I suspect that I'm probably doing forks on GitHub wrong, or at least I'm trying to use them in ways not envisioned by GitHub. In some cases I want to maintain copies of a GitHub repository for archival purposes (e.g., I'm afraid that the developer or GitHub will revoke public access to the repo), while in other cases, I want to institute a kind of code review process prior to merging upstream commits (e.g., I'm afraid of upstream doing something malicious). I will occasionally create branches in my forks, fix something, and send pull requests upstream, but I never feel like I need to maintain those forks---I'll happily delete branches or delete and re-create forks as needed.
I know many don't like the 'dirty' history, but I like knowing exactly how the updates made it into my repo.
But it's not the argument made by the author :-/
This is combined with most of the "historical record" really being worthless garbage: does it matter that you had to implement 12 fixups at various points and rewrite the whole feature thrice after requirements changed? Probably not. Does it need to be enshrined in the project's commit history? Absolutely not.
I imagine that people must be doing something involving git history which I simply don't have reason to do, or this "make your history pretty by rebasing" meme wouldn't keep floating around, but I really can't imagine what that activity might be.
For me, it's the ability to use git blame and determine which commit was responsible for a line of code and read the commit message about why it was added.
If you don't keep a clean commit history, you end up with a commit message like "fixing some issues based on comments" which affects at least 30% of the lines of code in the file. If I'm looking at that file 6 months after the fact, that commit message gives me no information about why those lines were added/changed.
With a clean history, you can use the same command and see the exact reason why a line of code was added through the commit message that added/changed it.
The code is the thing for me. And the less of it, the better.
In practice, I've never done this. I think it's that I work on a smaller codebase, or that I am 1/3 to 1/2 of the developers, and so it nearly always has seemed easier to read the code + tests, and poke at values in a debugger, than to use binary bug search. If I were on a larger team, or a larger project, `git bisect` might be more useful.
But I'm in your camp: I work with teams using git (and GitHub as a central repository), and a non-linear history has literally never been an issue. Even ugly graphs 7-10 histories wide aren't an issue. We also don't do pull requests (because we would we?).
We don't have a hard time with code reviews just using GitHub's commit history or even Gitk. In fact merge-commits often sometimes useful as markers of when the developer integrated his/her changes.
Admittedly, I'll occasionally do a rebase of upstream changes before I push if I know the change I made was small an isolated - but I certainly don't encourage other developers to do it.
I want my git history to be like my server logs, verbose. I can use tooling to reduce the noise and find what I need.
If you want to come back later during a git bisect and find out exactly when that bug that just took down your production application was introduced, then yes, absolutely. I'd rather look through 12 small commits than one monster one for that use case.
Look at it this way - a bisect cuts the search space in half with each progression, so you can double the number of commits being searched with the relatively minor cost of adding one more call to bisect when bughunting.
If you're tagging your milestones appropriately and otherwise practicing good repo hygiene, having n number of extra commits for any value of n is a feature, not a bug.
I'm not following this at all... what is it you need from this tool and why?
> Does it matter that you had to implement 12 fixups at various points and rewrite the whole feature thrice after requirements changed? Probably not.
Yes, code review is an invaluable part of the git history. It explains why you made the decisions you made. And having many small commits rather than 1 massive one makes it much easier to revert bad commits.
This is interesting - even places I've worked that have cared about commit messages have been pretty happy with the entirely content-free "Fixes from code review."
Do you summarise code review discussions in the commit messages?
> The whole "prettiness" thing seems like a bit of a fetishism more than about productivity.
When you say:
> I've never gotten an explanation that satisfies me
You have gotten explanations, you just pass judgement on those explanations and assume they are based on a fetish. You effectively wave your hands and dismiss them.
So, you are passing judgement based on your own admitted ignorance. And you go out of your way to hinder any investigation you claim to perform.
Whatever. You suffer for your self-inflicted and self-admitted ignorance.
> If you add a new member to your team, they would have to fork the repositories on GitHub, clone them locally, make the changes, push to their own fork and then create the pull request. With Gerrit, you could clone the main repository, do your change and then push it directly to the same remote as you cloned it from.
The objection would be that you can always push to a branch and issue a PR from there in GitHub too. Those steps are identical for both tools in a professional setting.
There are reasons for Gerrit but this isn't one.
You do need to create the pull request, but that's no different than pushing to a special magical ref.
I stopped reading exactly there, I knew that something was amiss. We followed that workflow at my last job, it worked well.
> As soon as someone added changes to their pull request – either by rebasing in the new changes or making it as a new commit – you lost track of the comments in the code and viewing what had actually changed since the last update became really hard