Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Idiot Proof Git

softwaredoug.com

Post reply on HN
Print view View original

191–200 of 435 posts

Re: Idiot Proof Git

#191

Post by theptip »

danwee wrote: post #130

Git commit crafting (and rebase to achieve it) is overrated. If you care about crafting beautiful series of commits so that the future readers understands what's going on: don't. Context is more useful to find out why something changed. Example: - you build feature F that is touching N files and M lines of code - you craft your git commits so that each of them is atomic and "understandable" on its own - now if I want…

I think you are arguing against an extreme version of the practice.

Normal advice is to shoot for 100-200 lines in a commit, not split that into ten commits that each change 15 lines.

If someone is splitting 100-line commits into 10-line commits, I would advise not doing that.

However the direction that people normally err (IME) is submitting 500-2000-line commits which conflate multiple atomic changes. I would also advise not doing that.

It’s easier to review a sweet-spot commit, and it’s easier to understand when you later have to debug/bisect a change set.

Another related concept is to try to split “functional no-op” refactors from behavioral changes. This is usually the first and easiest way of getting your commit size down, as refactors often bloat the diff.

In your example case I’d hope you have UTs exercising each chunk that is added. There are sensible APIs to shape at granularities smaller than the whole feature (model, service, etc) (or should be at least). If you really can’t add a new endpoint without a 1k- line PR I think you might need a new abstraction layer. But often you can add a new endpoint in meaningful chunks that are feature-flagged off, if you craft it thoughtfully.

Re: Idiot Proof Git

#192

Post by opsy_ »

agentwiggles wrote: post #41

I guess this will come off kinda... douchey? But I just don't find Git to be that hard. I know there's a lot of complexity there, but I find that 95% of the time I'm just git add -p or git add . and then committing. Every once in a while I'll do a rebase, and that's the most complex part of Git that I use with any frequency. I remember when I first was introduced to Git I found it confusing, so I'm sympathetic to new…

I think (from working with git newbies) the core difficulty is not understanding the Git's fundamental data structure. If you have computer science training / data structure understanding, once you realize (1) Commits are nodes in a graph (2) branches are just pointers to a specific node, you're off to the races.

From there, I find it straightforward to conceptualize all the different commands. Commit/push/reset/merge/pull/rebase/etc are all just different graph manipulations. There's usually multiple ways to achieve the end graph you want.

If you don't understand the data structure, you have no good mental model and it's just a series of ritual. If something goes off the ritual path, you're in trouble.

Re: Idiot Proof Git

#193

Post by xmprt »

dullcrisp wrote: post #187
RadiozRadioz wrote: post #134

Earlier quoted context omitted.

The reason people on here tend to dislike those types of tools is because they've probably been the ones who had to fix the tangles people get themselves into by using those tools. Tools that obscure details in favor of simplicity are fine in some cases, but version control is an inherently complex problem domain where having those details is important. In my experience mentoring juniors new to git, those who are jus…

> they probably don't know about -a How are you going to keep them from learning about -a?

When I first learned git, I had no idea what staging meant and the differences between A, M, D, R, AM, etc...

Only some time later (albeit a short time) I learned about -a, at which point I had a bit more understanding of the statuses and what it meant to stage a change. If you gave me -a before that, I would have never understood those things properly.

Re: Idiot Proof Git

#194

Post by OkayPhysicist »

opsy_ wrote: post #146
peteradio wrote: post #11

Earlier quoted context omitted.

Couldn't you rewrite history locally on your own branch and nobody would know?

Sure, unless you bork your local repo enough you need help from a teammate getting your work into a PR. Not to say that doesn't make for a good learning moment.

At some point, every junior is going to mangle something, and need a senior to sit them down and give them the git reflog talk. It's an inevitability, and should be embraced as a natural part of the evolution of a developer.

Re: Idiot Proof Git

#195

Post by groestl »

evandale wrote: post #122
alexb_ wrote: post #48

What's the deal with squashing commits anyways? I'm genuinely asking, because I've only worked with "squash everything before you put it up for review" but have never really figured out why past "it's what we've always done".

In my experience the squashing crowd is much louder and cares more about squashing than the crowd who prefers to see every commit. I hate squashing, but I've been beaten down into doing it many times because in the end it doesn't actually matter. The merge commits everywhere argument falls apart if you use log --no-merges. The shitty commit messages argument is solved by not allowing shitty commit messages. The "fixe…

> is solved by telling people to not do a million commits like that

> don't allow the useless "fixing typo" commits

This sounds like a much much more heavy handed approach than: I don't care what you do on your feature branches, just squash your commits to master and write a nice commit message explaining what you did.

IMHO, all your rules do is increase inertia of people to fix typos.

Re: Idiot Proof Git

#196

Post by ajorgensen »

cube2222 wrote: post #169
cnity wrote: post #155

Earlier quoted context omitted.

> commit crafting is overrated. For you, in your use case. When I look at my neat history and use git bisect, I get plenty of value out of it. People keep telling me to stop rebasing. I keep ignoring them; nothing new here.

This is basically solved by squashing each PR on merge and having a good PR title + description.

But why not do both? I find that nice commit messages make it easier on the review to see whats going on rather than a bunch of wip commits. Then squash all of the commits into a single commit with a nice title and message. The benefits of the squash at the end are, assuming you require tests pass before merge, you have a history of commits all with passing builds which makes bisect possible.

Re: Idiot Proof Git

#197

Post by groestl »

Izkata wrote: post #188
evandale wrote: post #154

Earlier quoted context omitted.

You'll have to elaborate with specifics because in my experience it doesn't matter if there's 100 or 10000 commits - git bisect works great in both instances.

It works, but it works better when you have the original 10000 commits. You can tell exactly what the committer was attempting when the bug was introduced. It may have been as a fix to something else, it may have been a typo when linting, it may even have been been intentional and the bug report is wrong. Other comments I made on another recent git post: https://news.ycombinator.com/item?id=33395616 https://news.ycom…

It may be the last thing they did on this day and the commit message reads "wip".

Re: Idiot Proof Git

#198

Post by aulin »

willio58 wrote: post #150
aulin wrote: post #137

Earlier quoted context omitted.

If you keep merge commits you can get the full diff at once and see all the context you need, if you don't you can still write meaningful commit messages that identify the feature you're working on so that in the future you can still do a diff between the first and the last commit and see it all at once.

Yeah if you just use commits as they are intended then the complexity of git drops off massively. I’ve used got for a decade and want to know how many times I’ve rebased? Zero

I tend to stage everything I do in my local branches, experiments, hacks, partial changes I don't have time to finish right now, unfinished refactorings. No way I share that without rebasing it and cleaning it up first.

Re: Idiot Proof Git

#199

Post by Izkata »

groestl wrote: post #197
Izkata wrote: post #188

Earlier quoted context omitted.

It works, but it works better when you have the original 10000 commits. You can tell exactly what the committer was attempting when the bug was introduced. It may have been as a fix to something else, it may have been a typo when linting, it may even have been been intentional and the bug report is wrong. Other comments I made on another recent git post: https://news.ycombinator.com/item?id=33395616 https://news.ycom…

It may be the last thing they did on this day and the commit message reads "wip".

And if that's where we end up while bug-hunting, we know it was caused by an unfinished thought, is akin to the typo case above, and should be fixable without much concern over what else it may break since it wasn't introduced while fixing something else.

Re: Idiot Proof Git

#200

Post by lelandfe »

t43562 wrote: post #180
lelandfe wrote: post #144

Earlier quoted context omitted.

Pull requests vanish when repos change hands. If you leave unique information in PRs, that information may be lost in the future. This has happened to me at 3 different companies now, where we inherited another company's code base. Keeping commits self-contained is the only way to future proof your explanations.

IMO one should maintain a CHANGES.md file with whatever you would be putting in the PR description in it. There's no specific need to squash commits to do this as long as you create it just before you create the PR. Even the odd bugfix or review comment after that is no big deal.

I don't see the advantage of doing this over ensuring the commits stay atomic, and I see several disadvantages.

Namely, git conflicts will happen constantly; and the file size will graduate from unwieldy to unusable over time. One repo I work on has >45,000 PRs merged. Good luck even opening that file.

This approach also poses risk: people will forget to add to it.

Post reply on HN