Using Git Wrong
dpc.pw
dpc.pw
> This is how the Kernel team does it, so everyone should do it that way
99% of the time, your team has absolutely nothing in common with the Kernel team and it's foolish to adopt processes from other teams without knowing why.
If anything, I would say the right way to use git in a small team is to always merge and never rebase.
Whatever you decide to do, make sure to think about your problem first, then use the tools to solve your problem.
The article has a valid argument with a vapid premise.
> This difference might initially seem rather insignificant, but it affects the whole software engineering culture.
Totally incorrect. I was a late git adopter and would describe myself as a fatigue whiner[1].
The main things that encouraged me to adopt git were the rebase and bisect capabilities. The decentralized aspects had absolutely nothing to do with my adoption of git. The author of the post is basically claiming that I have no idea what I am doing because I do not agree with them. Fuck that. This trash article doesn't belong on HN. It doesn't even promote good critical conversation.
[1] https://github.com/yarnpkg/yarn/issues/1136#issuecomment-316...
That said it can be useful sometimes to push changes out of band straight to a colleague. I have used this a few times to great effect but don't often come across the need
> git was invented before continuous delivery
git was invented 2005. So you mean when people used the term Continuous Delivery more frequently? Because using CI and automatically deploy is not "new" and was surely done before git - I remember doing that even with CVS and SVN.
It's goes hand in hand with BDD and automated testing. I reckon there's a lot less people doing this stuff today in the wild than you'd think.
Sure there's the Netflix's n googles of the world. It was only a few years ago I would consult on projects that would have no CI. It happens.
The same thing is happening today, just because you read about kubernetes or swarm simply does not mean anything like the majority of the people out there are using it in production. Just go to a conference and and watch people stick their hands in the air.
Sure SOMEONE might have been doing continuous delivery back in 2005, it certainly wasn't me. I was too busy learning about data access objects vs ORMs and releases were 3 months away if you were lucky.
Maybe before the word was invented, we just called it "developing in the production environment" before "continous delivery" was trendy. (And yes, we used the derisive name even if it took a couple spins on a developer workstation before getting rushed to prod.)
releasing often from one stable master branch
Subversion has serverside hooks. It was common to use them to push trunk to production after checkins. I know this because I was there writing some of those scripts.
For example, maybe in one project most of the time everyone pushes and pulls from a centralized master and not to and from each other. But then two developers get together to try and solve a particularly troublesome bug. While they're doing this, they can push and pull directly from each other without worrying about master, and then once they're done, they can merge, clean up, etc., and then push the result to master. Git allows these kinds of pockets of decentralized development within an overall centralized workflow, and does so seamlessly.
Since Git allows any organizational structure, it's up to the people working on a project to choose what works best for them. That's a responsibility that they didn't have before DVCS, because only one structure was supported by the tools.
But if they’re in different companies, and there is no central repo visible/accessible for both devs, then yes the D in DVCS is definitely appreciated.
>Have you ever did a code review, and gave feedback that was longer than doing the fix yourself, while you’re at it?
But this seems to neglect the utility of teaching people to do things correctly. Sure, I could always just fix junior programmers' mistakes, but it seems better to educate them in the first place.
Sending the fixes back down doesn't seem like it would have the same impact.
I say this, unfortunately, from experience. It has become common in our team for a new developer's first commit to be met with a blasting review, telling them everything they've done wrong, and most of it is capitalisation/whitespace etc. stuff.
Funny way to state that. There is this formatting tool `indent' for C and C++. It was already well known and rather rarely used back in 2003. Code formatters have not happened to "modern development" by any stretch.
> * You have one online repository for every project (or even the entire code in case of a monorepo).
> * Your repository/repositories have one branch that everyone branch from and develop their changes against. (Release branches don’t count.)
The kernel has one main repo that integrates things from the others, the "torvalds/linux.git":
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/lin...
This keeps Linux together as One Thing. This is Linux's CVSROOT, so to speek.
> You have one process that everyone have to follow to review the code; one review system and one place that runs all your tests.
No kidding; also: one payroll from which everyone gets paid and one product that customers understand: not seven versions of the product based on whose cubicle it came from. See the trend?
And most users use stable trees or distribution forks. So Linus's tree has the distinctive feature that almost no one uses it, unlike a traditional CVSROOT.
My understanding of the general flow layout for the kernel is:
developer branch -> patch -> mailing list -> sub maintainers -> large maintainers -> Linus -> Stable branch X.X -> distribution (i.e. Debian/etc).
With some things cherry picked here and there that don't flow through this, but that most code does.
- normal CVSROOT: developers develop on top of the head of the day.
- Linus's tree: developers develop on top of someone else's head of the day, except for some bugfixes.
Linus's tree becomes "similar" to a CVSROOT only after he cuts a release. But at that point, version control is actually not part of the game anymore.
Not everyone wants a distributed version control system, and not every team is ready for a more complicated process.
I always thought this process was sort-of intended. I feel it shows some respect to the author of the code under review to comment on it, and not to change it–especially not if that would result in changes that are then merged without first getting approval from the author. I'm having trouble finding the right words for this idea...
It's also a way to teach people about coding standards and expectations for little things, such as spelling in comments.
Agreed. It also prevents the reviewer from introducing extra bugs from knowing the code less than the author.
Code checked into version control is never changed. What happens is the reviewer creates a derivative work. If they use my code as a basis for their derivative work, that's a sign of respect just as much as anything else. It means that they think I actually got it mostly right.
The same argument applies if it's merged right away. Unless the reviewer(s) blatantly misunderstood something important, it's likely their fix is an improvement. And even if they did, the previous code is just a reset/rebase/oops-commit away.
That said, you should still discuss the code to iron out misunderstandings and learn things.
Distributed source control is awesome. But closed source Devs don't use git for that feature. We use it for fast local branch and merge. We use it because it's way more ergonomical than cvs/svn. We use it because it works just as well for distributed development as it does in the single source of truth setup that we need.
I think the author has a great point about the differences between how git was "designed" to be used as compared to how it ends up being used in orgs most often. But I also balk at being told I'm using an open source tool incorrectly as a general rule.
disclaimer - I really want prime emporer of gitland to be a proper honorific.
Lets pretend that architects are under team leads, seniors under architects, juniors under seniors, etc.. In this hierarchical model every layer forwards some responsibilities down the chain and trusts the task to be broken down, distributed further down and completed. In this model code reviews function merely as tools to roughly check for gaping mistakes and whether code fits "architecturally". People are trusted to do their job to the best of their ability and are allowed to have personal style.
In a model where seniors go over everything juniors made, fix according to their own view and then commit we have a problems that lower layers are not trusted to do their job correctly and in the end everything has to strictly conform to product owner's vision or will be "fixed" (rewritten). In this model there are no longer junior, mid-level, senior engineers, because the more minions one has under them, the less time they have to produce anything on their own. In this model there are junior, mid, senior code-monkeys and one mighty product owner who knows it all best. Not sure that environment where only career growth opportunity is ability to rewrite other's code to your own liking would be good for team morality.
Of course no such thing exists, so I use Git which has these features but combines them with 10x worse UX and 10x more condescension from the community. But I put up with that, only because it's the lesser of two evils.
I can't imagine I'm alone in this.
For example (and simply thinking out loud so don't shoot me), why can't I modified a file. Then modify another file. Then create a commit object (so to speak) and drag the files on to that commit? And then, once I'm sorted, I drag the commit on to a push. Which perhaps goes into a queue for review. And then the tool somehow help enable that review knowing any other commits on the same files, etc.
Coding is building. Seeing it visually is going to help. It always just seems to me I'm forced to think too much about Git, when a UI would do. And the brain cycles I use on Git (however small) could be used to write code (not wrestle with managing it).
As with Subversion, when you're not working on a project alone, you normally run Fossil on some central server somewhere. Then because it's a DVCS, you clone a copy of the repository to your working machine.
So far, it's vaguely like Git, but it diverges quickly because the default behavior of Fossil from that point on is basically identical to Subversion, only with a much saner branching model and more features. (Built-in wiki, ticket tracker, web UI...) A checkin on the local machine first checks the change into the local repo clone, then it attempts to sync the local repo with the remote repo it was cloned from. (That's a two-way sync, not just a push.) Thus, like Subversion, checkins normally immediately appear on the central server as soon as they're checked in.
The sync can fail, of course, but what happens then depends on why it failed.
If the sync failed because the tip of the branch you're working on has moved forward on the remote repo, because something else has been committed to it in the meantime, it aborts the checkin because to proceed would create an unintentional fork in the branch. So, it makes you do a pull first to merge the remote changes into your branch. You then re-build, re-test, and then check your changes in as the new branch tip. (You can force Fossil to accept it anyway, but then you have to deal with the forked branch.)
If the sync fails simply because the remote server couldn't be reached, your changes are checked into the local repo clone only. Once connectivity to the remote repo is re-established and you cause a sync, it pushes your local checkins to the remote server. Unlike the online sync failure case, Fossil won't refuse to sync your changes up to the remote even if it creates an unintentional fork, because otherwise there would be no way to sync. You must then resolve the the fork manually, which typically is done by selecting one of the fork tips as the new branch head and merging the other in. Alternately, you could call one of the fork tips a new branch, then merge later.
Fossil doesn't care which way you do it, because branch names are for the human only anyway. It wants you to resolve the fork only so you don't confuse the other humans, not to solve any confusion on Fossil's part. (Underneath, Fossil only cares about the relationships between the artifact hashes, not about human-friendly file and branch names.)
You can avoid the forked branch problem by checking changes in on a new branch when you know the remote repo is unreachable, such as when you have no Internet access and the remote repo is on a public server somewhere, or when you're working away from the office and the remote repo is inside a firewall and you're on the outside. Pushing a new branch never creates a fork.
If you don't like the autosync feature, you can turn it off, so it doesn't even try to sync changes between the local clone and the remote it cloned from on checkin. In this mode, it works more like Git, only with the day-to-day working sensibility of Subversion. You can then do manual pushes once your local repo is in a shape you feel like sharing.
If by "sharing with colleagues" you mean that you and your colleagues do not have mutual access to the same central repository, Fossil can cope with that, too. Fossil has a feature called "bundles" that pack up a subset of a repository, which can then be merged into a different repository. You can only do this if there is some history between the repositories, so that they at least share the same root — you can't just graft pieces of one repo onto a totally foreign repo — but it does let people who otherwise wouldn't have checkin rights on your repo send you changes. Because the Fossil bundle format preserves 100% of the Fossil feature set, it's basically an uber-patch(1) format, preserving things like checkin comments, branch points, file renames, etc., which unified diffs do not.
Git has some advantages, but you have to really need them bad to put up with the UX problems you mention. Examples:
1. Popularity. If you need tool integration — really need, not just want — then Git is more likely to be supported by your tools than Fossil.
2. GitHub. Some people's concept of "Git" is actually "GitHub". Fossil's web UI has a large subset of the features GitHub adds to Git, but if you need something Fossil lacks, such as the network effects stemming from GitHub's prime position in the software development community, it's fair to regretfully say you cannot use Fossil.
3. Rebase. Some people really really need this. Many more people think they need this. Fossil users prefer that a DVCS work more like an accounting system, which records what you actually did, not what you should have done, edited after the fact.
4. Working in private by default. Personally, I think this causes more problems than it solves. It encourages people to go off on wild tangents rather than collaborate with their peers. This mode of operation is essential in a development community the size and distributedness of the Linux kernel, but very few projects have this same pressure to decentralize. If you know all of your collaborators by name, you probably don't want everyone going off on their own wild tangents. If you can't safely commit to the trunk, check your stuff in on a branch so it doesn't break everyone's build, then merge when you're ready. Meanwhile, your coworkers can see what you've been up to, so no one's surprised when the branch finally gets merged in.
Many of the suggested methods of code sharing do happen, especially on large projects with multiple teams.
But the “one master branch” problem is not a result of missuing git... It's a result of there being one version used in production... No git process change is going to change that.
His real issue seems to be with code reviews... Which isn't really something unique to git...
This applies to pretty much any version control where you can create a remote branch.
Seriously, the author probably never worked in teams with different sizes [1, 2], considered another workflows [3, 4] and skill levels of the developers (yes, that's a variable you have to consider also!)
Also if you use Git in the same manner as CVS, you should do it because it's much faster, more reliable and more flexible...
Edit: Ok, the author is saying things which I agree with, I was commenting on the click-bait title and the first paragraphs mostly, but still, there is more to the story.
[1]: https://m.signalvnoise.com/the-majestic-monolith-29166d02222...
[2]: https://blog.bradfieldcs.com/you-are-not-google-84912cf44afb
[3]: https://trunkbaseddevelopment.com/
[4]: http://nvie.com/posts/a-successful-git-branching-model/
This is also where the continuous integration tests usually take place.
What is a good point of the OP is that people may not be sharing and testing code enough before pushing to the reference repo.
We also have the same at my place of work, but for convenience when requiring to patch production - ie just branch master rather than scramble to find the commit hash last released. It's not a requirement, but it definitely makes life easier.
It is easier and safer to have n developpers synchronizing to one reference repository, than to keep n developpers and repositories synchronized with each other.
Code review often catches typos, or edge cases that should be tested. As the OP points out, you end up serializing the problem into a comment, which the other developer has to deserialize, implement, and send back. Worse, these sort of issues often come across as nags - I'd be happy to fix a typo myself, but feel like a jerk calling someone else's out. I rather like the idea of pulling down somebody's diff, amending it, and pushing it back up. I'll have to try that sometime.
Also, if you do this people will likely subconciously take advantage [1] of it in the long run, and if you ever leave the team quality will suffer.
[1] ever had flatmates? Do all the dishes and cleaning for a month and you’ll see :-/
Talk about throwing out the baby with the bathwater!
Just because you have a blessed master branch, and a central repo, does not mean imply you aren't making use of your DVCS. You can still do all development on topic branches, including CI, and you can do it all in an asynchronous manner.
This idea of pushing branches between individuals in an ad-hoc fashion with whatever tools everyone wants to use makes perfect sense for something like kernel development where A) there is no single product running on a single production environment, it's a freaking kernel deployed to billions of systems globally and B) you're dealing with a huge set of people who you could never hope to standardize anyway.
When you're shipping a complex product to a single production environment, it makes a great deal of sense to centralize certain things. Having everyone bike-shedding on a million different code review paths, and relying on each individual to pass their code around to other individuals with no single place where you can look to find out what someone was working on will lead a lot of chaotic inefficiency that will leave juniors confused, and all in the pursuit of some platonic ideal that is cargo-culted from a completely different project.
Don't get me wrong, I think most people have a lot to learn about how to use git better, and I am a huge proponent of the value of a well-curated VCS history, but this article is inappropriately prescriptive nonsense.
Yes. Yes yes yes. I hate this so much. I want my code reviews to involve fixing little things. I don't even have suggested edits, I can only highlight comments and make comments, and even suggested edits wouldn't be enough. I want to just make the changes. It's all on a branch anyway.
99.9% of code review that I do, and that I get, is not training. So maybe don't make all the changes for new employees? I use vim. Most people at my work use vim. Our build process is complex. Everything we do is really complicated. It works for the experienced, yet it has a learning curve.
So, new people get a slightly different experience. That's fine.
> open in IDE/text editor of their choice with or along diff viewer, jump around the new code, run tests and even write some new ones, toy with it
This doesn't scale. I know team leads in large projects where reviewing external contributions is their main responsibility; this style would be impossible.
Instead, I think the human, diff based approach with CI running against the reviewed code is enough. A reviewer going over the tests which are shown to be passing should be sufficient after the usual diff read-through.
Just because (autonomous) you can merge and push, doesn't mean you should. Right?
>It might seem like chaos, but it is not. It is just fluid synchronization, or to put differently: eventual consistency system.
Can anyone explain this? The example he used wasn't fleshed out enough for me to understand how this system wouldn't be chaos