Abandoning Gitflow and GitHub in favour of Gerrit
beepsend.comGerrit is the wrong solution for truly agile software development. I had a client ask my team to use it, and it was a real PITA.
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.
> 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.
Your argument would be better received sans-ridicule.
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?
There's nothing ridiculous in xyzzy_plugh's argument.
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.
"Look at this guy mispelled the name of something. He must not know how to identify ridicule"
It's also possible that they used Gerrit before those features were added which was maybe two years ago.
That's funny, as someone who has used both github and gerrit for several years each, I completely disagree. I found the gerrit flow worked well.
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.
> The fact that one commit = one merge is ridiculous. It encourages monolithic commits for no reason other than that the tool demands it.
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 use Gerrit everyday (I work on MediaWiki) and I'm not having any problems submitting multiple dependent commits. Yes, you need to rebase the newer ones if an earlier one was non-fast-forward-merged or if an earlier one is amended, but most of the time (if there are no conflicts) this is just a click on the "Rebase" button in Gerrit's interface.
Gerrit is the reason I rarely submit MediaWiki patches back to the Wikimedia Foundation. Gerrit is a giant pain to use, has a terribly designed interface, and requires a command line plugin that makes it trivial to screw up patch reviews. I can not ask other developers on the team here to do it either since I can not justify having them hours just to learn how to not screw up using Gerrit.
The git-review plugin is not required, you can instead do `git push gerrit HEAD:refs/for/master` to push changes for review and use the snippets from the change's page to pull them. But it's easier to do `git review` and `git review -d` instead, and it doesn't require understanding or memorizing the weird push command. (I work on MediaWiki, by the way.)
I used Gerrit for over a year at one company and I have to agree. Every minor change required amending a commit and another review. It really breaks a lot of the git process.
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
Glad to hear you like GitLab! And if you want something more formal GitLab EE has merge request approva; https://about.gitlab.com/2015/06/16/feature-highlight-approv...
The article isn't particularly well written or argued, but it does have a core of truth to it: serious code review in GitHub is painful. However jumping straight to Gerrit to solve that problem seems like overkill to me. Sure, you get a really powerful and extremely configurable code review system, but you have to retrain for a new (and honestly a little long in the tooth) UX and spend time administrating the system.
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.)
Just use Phabricator! It's the best code review system I've used so far. Many large open source projects and companies have adopted it.
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.
Phabricator is nice, but it's more of a full-featured replacement for GitHub as a whole. Some people want to keep most of GitHub and just improve on the code review aspects.
> 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.
> Phabricator is nice, but it's more of a full-featured replacement for GitHub as a whole. Some people want to keep most of GitHub and just improve on the code review aspects.
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.
Amen, Phabricator all the way!
Nothing about what was wrong about gitflow.
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 ?
> 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?
> 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.
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.
Within gitflow it is definitely necessary to have an integration branch - it doesn't really matter if you call it develop, or you progressively merge different features branches down together, or group them into one feature and just develop them all on one feature branch from the get go, the result is the same - you end up with all your code integrated on one, not necessarily always stable, branch.
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.
> It's necessary to have a less-stable, separate,
...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.
> Why shouldn't I call my branch for integrating code... integration, or something?
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!
A common case where this doesn't work well is embedded code. You'll frequently have many different releases of your code "in production" at the same time.
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.
That makes sense, but I don't understand how renaming the branches made that problem go away.
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.
Jenkins should always pass master. develop is for things that need to be done but necessarily mean Jenkins is at least temporarily very unlikely to succeed and thats "OK" or even "normal".
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.
Note that you can change the default branch in github:
https://help.github.com/articles/setting-the-default-branch/
I fully agree with using master for development. I also think that the git flow process is over the top for most teams. I detailed how to adopt just the things you need in GitLab flow.
"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.
At work, we use master as you describe, and follow gitflow for feature and bugfix branches.
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.
I really want something that provides better code review than GitHub. The described code review features of Gerrit sound promising. But the article says you can't submit a series of commits for review as a unit, you only submit a single commit. Is that really true? That seems like a rather awful limitation of the system. Sometimes my changes work well as a single commit, but often, especially when doing more complicated things, it's much more preferable to use a handful of related commits, all of which should get reviewed and merged as a batch. Does Gerrit not support this?
With Gerrit you can chain up a series of dependent changes. The OpenStack Developer's Guide summarizes the workflow pretty well - http://docs.openstack.org/infra/manual/developers.html#addin....
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,...).
Phabricator[0] is awesome. Diffs (like PRs) contain several commits, but are reviewed, discussed, and "landed" as a unit with an auto-generated commit message referring to the Diff description and a link to read its history, but "master" is a linear sequence of Diffs being landed. You can also configure all kinds of rules like "X person must sign off on any changes to this file" or "do not merge code to master unless it is approved by N people other than the author." The UI is a bit clunky but I love it, and it's open source and very flexible.
I used phabricator at my last job and was really impressed. Now I'm using Gerrit and I'm less impressed (but still getting used to it).
You say clunky, but Phabricator's UI makes Gerrit look like a WAP app.
When you say a linear sequence of Diffs being landed, do you mean it commits squashed merges, or does it create actual merges (so the history of master includes all the commits in each Diff)?
AFAIK this is configurable. We did squashed merges, which I quite liked, because "git log" for master read nicely but you could go look up the more granular history if you wanted to.
hopefully it's slightly less clunky today, just wrapped up the semi-annual refresh this past week. Still... we have our fair share of "-isms".
You might want to check out https://reviewable.io. It has most (if not all) of the goodness of Gerrit, but is trivial to set up (SaaS) and integrates smoothly with GitHub. Every PR becomes a review and gets automatically updated whenever you push to the branch.
Disclosure: I'm the founder.
Worth noting that Servo uses Reviewable, if someone's looking for an example of its use in a large, active project with many contributors: https://github.com/servo/servo/pulls
Wow, the described feature set sounds pretty good. I'll definitely look into this.
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.
Sorry about the mess on the demo review -- since everybody gets write access to try things out, it tends to get messy over time. I just reset it now so it looks clean again, and should probably just stick the reset script in a cron job...
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!
Thanks, the demo review is now much easier to read.
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).
Our team started using reviewable.io a few weeks ago, and I quite like it so far. It solves the "squash/rebase->force-push->lose history" problem with Github PR review.
I've been working on a code-review tool that does exactly what you want: https://www.omniref.com/code_review
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...
Looks promising. Does it handle force-pushes to PRs? It says it doesn't lose comments when pushing new commits, but it doesn't mention how it handles history rewriting.
It doesn't lose track of state when you force push. You'll see the old commits in the history, and the force-pushed commit, and the comments will still be on the old commits, the issues will still be active, etc. This isn't perfect, but it's a start, and it's better than GitHub.
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 can push up a series of commits, but each one is it's own code review so you want to do it less often. Another thing is if you have to add something to the HEAD^^ you have to rearrange your history and squash things together, but in the end the history is really clean. Also, each push of of each commit is versioned so you can easily tell what has changed between the last code review and the current; this is impossible with GitHub if you want to keep your history clean and compare different versions of the code review.
I'd suggest you take a look at phabricator too which has great code review and issue tracking. It does unfortunately still lack support for a first class notion of a patch series.
It does. Dependant changes/commits can be pushed at once, and if they are submitted out of order, they will be queued until the dependant changes are submitted/merged. In recent gertit releases, you can even submit (merge) all changes in a particular topic at once.
"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."
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.
Exactly. There's multiple way of working together with Git/Hub. Gitflow is not a bible, but a customization workflow that should be adopted to the team.
There are far fewer reasons not to use Gitflow than you might initially think.
I personally don't have any issues with Gitflow but it seems the OP does.
> You don't have to do this with Github. Just clone directly to your local machine.
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.
Team members can also work in their own private branches that are pushed to a single repository - that's usually sufficient as a "backup".
That requires push-access. Not all team members have push access to all our source repositories.
Allowing someone to push branches to the "central" repository versus requiring them to have forks of it is functionally the same thing, assuming you set up branch protection for master.
> assuming you set up branch protection for master.
Bad assumption. However, branch protection sounds cool. Will investigate it.
honest question: why don't they have push access?
Prior to 2 minutes ago, I had no idea you could lock down a branch in github. We have folks working on our codebase, and the team wants to enforce code-reviews for anything that goes into our master branch. To enforce this, only a few people have push access.
This is similar to the open-source maintainer model.
Why would you not be able to use Github as a backup? Github keeps all the branches that are pushed to it, as any git repo.
If you have push access to the repository sure.
What if you don't have push access?
You don't give your employees push access to their repos?
By default, only admins have push access to all the repo's for an organization.
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.
I don't see anything in the article that goes against gitflow. Is it just me?
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.
This is way more complex to me than GitFlow with pull requests. As a matter of fact, if you use something like SourceTree for most of the initial steps it's a few mouse clicks. Also, try Gitlabs for your reviews; it's pretty good!
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.
What appeals to me about the voting scale is that it's well-defined: a -2 means "don't merge yet", a 1 means "looks good to me but I'm not sure that it's ready to merge". More of an enum than a voting mechanism.
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.
Voting at GitLab is mostly thumbs up for good code and leave line comments for bad code. We don't vote bad code down, downvoting is mostly for issues with feature requests that are controversial.
We tried Gerrit. We ran as fast as possible away. It seemed to hate merge commits, and would hang up often. I'm not sure if it's changed at all, but I think the only option was to review every commit individually inside of a branch. It seemed to really be pushing us towards squashing a branch and pushing that up.
Gerrit does favor rebasing over merging but that's hardly a reason to run away from it
Rebasing is great for version history but is hell for collaborating on a feature. If anything is the Achilles' heel of Git (aside from the groundbreaking levels of inconsistency in the CLI) it's this.
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.
Would you care to elaborate what you find wrong/bad about "parallel histories"? I'm curious.
This is related to the conversation about squash that happened last week.
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.
> 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.
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.
> This is related to the conversation about squash that happened last week.
> 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.
consider mercurial. You may be pleasantly surprised.
edit: spelling
What do you mean by "fixes parallel histories"? What is a parallel history and how do you fix it?
> I'm not sure if it's changed at all, but I think the only option was to review every commit individually inside of a branch. It seemed to really be pushing us towards squashing a branch and pushing that up.
The author mentions this as being a feature of Gerrit in the post.
There's a Gerrit service that integrates with GitHub:
- [Gerrit Code Review for GitHub](http://gerrithub.io/)
If only it was only code review. This is basically another GitHub git hosting service using gerrit, and it syncs to github for open source projects.
I don't want a new hosting service, I just want a code review process on top of GitHub.
Try https://reviewable.io then. Just code review, runs on top of GitHub, no extra repos to manage. (Disclosure: I'm the founder.)
Try Phabricator (http://phabricator.org/), you can point it at a remote repository without hosting it.
I'm confused. It syncs (some of) your repos and any PRs in them, so what else do you need to do with it besides code review instead of with GitHub?
nice one. Would be great to know if somebody has experiences with it?!
I use gerrit daily. It's a good tool once you learn it but as an end user I find it has some usability issues. I would very much recommend it though for teams. You'd need someone to instill good code review culture in your team. It dictates the +1/+2 review flow so you have to adhere to that for it to be a natural fit.
What I miss in article is for how long they are on it. Is author after peak of inflated expectations?
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.
I think there's something amiss in your development process if you feel you can merge changes that have -1. What's the point of a review in this case?
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.
We've been using it since May 2015. Vetos work well for us. We push new releases every week and often there are dependencies between the reviews (front-end waiting for a vetoed back-end review) which results in them being a non-issue (discussions happen frequently and updates are coming in quickly).
We've been using Gerrit since 2011, and it's worked really well this entire time. It does require communication if you have different features in different patches, but frankly, it's light years ahead of anything GitHub still has to offer in terms of pure code reviewing capability.
Without fail, every single one of these "Git Flow sucks!" or "GitHub sucks!" posts has a fundamental misunderstanding about one or the other.
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.
Gerrit is being used by many large open source projects,
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.
> 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 (almost impossible if the new push was rebased with the new changes).
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.
Blech. I have to use Gerrit at one of my current clients, and I fucking hate it.
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.
"At the time we had consultants working with us to speed up the development process..."
Mistake #1.
> 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 (almost impossible if the new push was rebased with the new changes).
Why are they all rebasing in their PR branches if it obviously makes the PR unreadable?
I don't really see what's different. Both github and gerrit have voting and you're still creating a pull request like in github. The only difference is that this pull request is restricted to a single commit.
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..
> 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"
> > 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"
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?
What I got from this article is idea to squash every pull req into a single commit. I think this is valuable idea.
I agree, which github now does for you
thank you for pointing this out for me
As someone who does a lot of git bisecting, this is a horrible idea. Only do it on a case-by-case basis. If it takes 5 commits to make 5 complete changes that implement something, keep them as 5 commits.
It's such a naive idea to apply this to every PR unilaterally.
Anyone know of a good hosted gerrit solution?
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.
try gerrithub
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).
I had the same impression reading the intro; I was done after reading about Agile and consultants. In every single instance I've seen these two combined, mediocrity followed. Even in the cases where they thought they had agility the results were still terrible and unproductive.
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.
A thought just come to my head... For a lot of dev departments at non-tech companies, might consistent mediocrity be an improvement? If management is used to disaster at every turn, and then they hire some consultants, and then things are just lousy all the time, might that not count as a legit win for the consultants?
That's what I meant by "overlooking the company's context".
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.
I've used gerrit and it's much better than github/bitbucket. My knock on gerrit is it requires one "very good" git person to be on the team however it makes it very easy to support a larger team and do cleaner code reviews.
This is not the only argument in this piece — I'd even argue that it's a minor one. It hardly makes sense to discredit the author and this article based on this GitHub misconception.
The free floating anxiety and dislike of the article is a symptom of an architectural/mgmt mistake in the article.
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.
My main point here is that on GitHub, making a pull request is super easy. There's even a cool pop-up if you visit the main page of a repo after pushing to a branch that asks if you'd like to make a pull request.
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.
Github isn't exactly a utopia of UX.
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:
Or something like that. I think I got lost somewhere along the way trying to checkout the upstream branch (is it "up/master" or "up master"? It depends) but it took 5-ish minutes.$ cd ~/src $ mkdir github $ cd github $ git clone myfork $ cd myfork $ git remote add up upstream $ git pull up master $ git push origin masterGithub 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.
It'd be nice if there were a way to automatically update a fork, including the issue tracker, wiki, releases, etc. The best I've come up with is a set of scripts that iterate over all branches of all forks, running `git checkout $branch; git merge --ff-only upstream/$branch; git push`:
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.
You can create a PR from the upstream to your repo, then accept that PR. (I know, that's not immediately intuitive, but at least you can do it without pulling a local copy.)
I tried that once but ended up with a merge-commit with my name on it in the history of my "fork". Is it possible to not end up with such merge-commits?
I don't think so -- you'll need to do something like a git rebase, and there's no way to do that via the GitHub UI.
I know many don't like the 'dirty' history, but I like knowing exactly how the updates made it into my repo.
nice reference to President Bush there at the end :) but I do agree, merging locally(when there are a ton of diffs/merge conflicts that I had to sift through) is still a cumbersome process.
That's really great point, and something I'd love GitHub to put some more time into.
But it's not the argument made by the author :-/
I'm not a git expert but I've contributed to repos which refused to merge the github pull request to keep their history clean. I mean, seriously?! So I don't know what is better...
Github just added a way to squash pull requests to a clean commit last week, so there's that
I've never understood why people think omitting a historical event keeps their "history clean".
Because I've yet to find a git graphlog tool which made viewing pretty nonlinear git history (as precise a historical record as they are) anything but a pain in the ass, some of them barely even manage to display a dozen concurrent "branches".
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've yet to find a situation in which I cared even a little bit about the prettiness of a git history. I am having trouble thinking of more than a couple situations, ever, where I cared about the contents of a git history, at all!
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.
> I've yet to find a situation in which I cared even a little bit about the prettiness of a git history. I am having trouble thinking of more than a couple situations, ever, where I cared about the contents of a git history, at all!
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.
Blame is occasionally useful, but for me more than half the time the file gets moved somewhere else or the indentation changes as part of some other change so you lose all context about the original commit.
The code is the thing for me. And the less of it, the better.
The argument made to me by a coworker (for why we should squish + rebase all PRs before merging) was that it makes it easier to use `git bisect` -- which is basically a binary search for Where a Bug was Added.
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.
What's the issue with merge commits and git bisect?
I think, based reading comments and articles from various sources, it's all because of GitHub pull requests. A lot of people seem to rather a pull request be represented as a single commit.
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 sort of feel the same way. The whole "prettiness" thing seems like a bit of a fetishism more than about productivity. Sort of like how people fetishize "inbox zero" (but in that case I do see the productivity gain).
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.
does it matter that you had to implement 12 fixups at various points
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.
> Because I've yet to find a git graphlog tool which made viewing pretty nonlinear git history (as precise a historical record as they are) anything but a pain in the ass, some of them barely even manage to display a dozen concurrent "branches".
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.
> Yes, code review is an invaluable part of the git history. It explains why you made the decisions you made.
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?
Yes, I do, I try to treat each commit like something that can be read in isolation. But nevertheless, the merge commit will have (if using GitHub) a reference to the pull request where you can see the discussion, which is valuable imo.
Have you had any feedback from colleagues that have found that useful?
Check out http://gitup.co
If you don't understand, then clearly you should investigate why as you cannot form an opinion on something you don't understand.
I have investigated, why are you assuming I have not?
Clearly not enough, because you don't understand. You literally said that. How can you pass a judgement on something you don't understand?
Who said I'm passing judgment? I a lot of people prefer that workflow so I'm guessing there is some merit. I just don't understand it (I've never gotten an explanation that satisfies me).
You do, in your own comments. You pass judgement. Take for example this:
> 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.
What's wrong about what he said? Which step can you omit?
It needs the context. I've quoted the whole thing below:
> 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.
Exactly, thanks. My reason for saying I'm done is that, if you haven't yet discovered that GitHub makes it really easy to make a pull request, you're probably not the right person to be criticizing it and proposing a new flow.
You don't need to fork the repository or push to your own fork (if you have write access to the original obviously, but gerrit seems to assume that), intra-repository pull requests are perfectly well supported.
You do need to create the pull request, but that's no different than pushing to a special magical ref.
Forking the repo
It's not that any of those steps can be omitted, it's that those steps overall are not difficult or complex.
Sure, but the author literally says "pushing to a special branch might seem more complex, but consider X." Those steps are more complex than pushing to a special branch.
Just last night I decided not to bother submitting a (tiny, one-line) patch to an open-source project because I didn't feel like going through all that rigmarole. The steps may not seem difficult or complex but they are steps, and they are friction, and eliminating friction is generally a good thing.
This is a pattern I've seen in teams to enforce review of code before it's merged. You can make the main repo read-only for most of the team and require PRs be submitted from their own forks. Then, someone with write access on main repo can review and merge the PR. I think this is overkill, personally, but just want to point out that maybe this team was employing this process, rather than them not knowing how to create PRs from a branch.
Sure thing - but that's a more complicated flow brought on by something other than GitHub.
Does the new GitHub code review tool handle rebased commits like Gerrit does via "patch sets"? That seems like it would be really useful.
I know they've talked about implement something similar. Since the Open letter to GitHub a lot have changed and I wouldn't be surprised to see GitHub support most of what Gerrit has (and more) in a near future.
> Annndddd we're done
I stopped reading exactly there, I knew that something was amiss. We followed that workflow at my last job, it worked well.
Heh, I literally had the same quote in buffer and was coming to make the same comment. Clearly they don't know how GitHub works.
Am I missing something? What the article describes seems to be the most common way people use GitHub with a git flow branching style. Is the contention just over the "would _have_ to" part? I think it's likely the author knows that GitHub doesn't _require_ this. And even if not, it doesn't seem pertinent. Doing a PR from a branch of [a clone of] the main repo would require basically the same steps.
Exactly why I'm frustrated with GitHub's PR:
> 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
Out of the frying pan and into the fire. I don't like Github but, from the sound of it, Gerrit is more complicated which is what I don't want.
I would never ever recommend gerrit to anyone.
+2
... still waiting for someone to workflow+1
One word for Gerrit: UGLY! :P
It is fairly ugly, but it also allows you to drop in your own css file. It's not a 100% solution, but you can go a fairly long way towards making it more usable.
yes but there's polygerrit project to improve that.