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

341–350 of 435 posts

Re: Idiot Proof Git

#341

Post by bacon_waffle »

gardenhedge wrote: post #281
cnity wrote: post #158

Earlier quoted context omitted.

If the employer I worked for started micro-managing the way I use my tools (that affects nobody else) I would consider leaving, honestly. If I rebase on a branch that hadn't been shared with someone else, why does it matter what my boss or team thinks about that approach? Code styles are one thing, what I type into my terminal is another.

When you commit you're sharing with your team and future team. I think it's fair to have a set of agreed guidelines around that. What if you wanted to put all your commit messages as "cnity did it"?

I agree with the post you're responding to, and make a lot more commits than I share with my team. Pushing commits to a shared branch is where work is shared.

At my current employer, we used to use Perforce, and in that world you're totally right that committing (submitting in Perforce terminology, IIRC) did share changes. In that context, we developed a lot of bad patterns, losing code or holding up other people's work while a developer got their work ready to share. Transitioning to git has been super painful, due mainly to people treating git as if it's the same sort of thing as Perforce...

Re: Idiot Proof Git

#342

Post by throwaw20221107 »

ratorx wrote: post #340
throwaw20221107 wrote: post #336

Earlier quoted context omitted.

Ok I agree, you roll back to the known working version. The easiest way to do that is revert the whole PR (or data deploy in case of flags, ofc). My point is not "flags vs. no flags". My point is "each PR should generate one commit because that's easy to revert". The commit dag of git is a cool feature but shouldn't be in `main`. It's so much easier to work with a linear history and one where each commit contains all…

Linear history is good, and having multiple commits in a PR doesn’t prevent it. The only change is adding n (ideally well crafted) consecutive commits rather than 1.

The problem is who is doing the crafting. (And the approving.) At least squash based approach limits to 1 the number of commits an untrustworthy "crafter" can occupy.

Re: Idiot Proof Git

#343

Post by bacon_waffle »

OkayPhysicist wrote: post #263
prettyStandard wrote: post #236

Earlier quoted context omitted.

Haha. what's the git reflog talk? To be clear I know what reflog is, but what's "the talk"?

Just the explanation of "no matter how bad you've fucked up your repo, as long as you haven't run 'git gc' or waited a few years, all you need to fix it is to find the commit hash from git reflog, git checkout that hash, and then git branch to give you an easily accessible reference to the commit". Followed by some pointers on how to efficiently dig through reflog. It's pretty freeing to realize that it's basically i…

Or made a "fresh clone". I've had colleagues nuke a repo when it gets in to a "bad state", start over with `git clone`, repeat the work that git lost...

Re: Idiot Proof Git

#344

Post by ratorx »

throwaw20221107 wrote: post #342
ratorx wrote: post #340

Earlier quoted context omitted.

Linear history is good, and having multiple commits in a PR doesn’t prevent it. The only change is adding n (ideally well crafted) consecutive commits rather than 1.

The problem is who is doing the crafting. (And the approving.) At least squash based approach limits to 1 the number of commits an untrustworthy "crafter" can occupy.

I think most code review already requires good faith on behalf of the reviewer already.

I do see what you mean about untrustworthy crafter. If we want to preserve master history, then the damage of a bad commit chain is worse than of bad code (which can be fixed/undone).

However, I think that the truly adversarial case is rare (and an exception could be made and master history be rewritten in that case). In most cases though, hopefully your coworkers and not deliberately trying to sabotage the codebase :). And I don’t think the commit chain needs to be a work of art or anything, just mainly avoiding typo commits and similar, so it shouldn’t be difficult to do when approached in good faith.

Re: Idiot Proof Git

#345

Post by throwaw20221107 »

ratorx wrote: post #344
throwaw20221107 wrote: post #342

Earlier quoted context omitted.

The problem is who is doing the crafting. (And the approving.) At least squash based approach limits to 1 the number of commits an untrustworthy "crafter" can occupy.

I think most code review already requires good faith on behalf of the reviewer already. I do see what you mean about untrustworthy crafter. If we want to preserve master history, then the damage of a bad commit chain is worse than of bad code (which can be fixed/undone). However, I think that the truly adversarial case is rare (and an exception could be made and master history be rewritten in that case). In most case…

> the truly adversarial case is rare

The problem is not adversarial, or due to malice. The problem is ignorance and expediency driven by a desire to push code and little incentive to cleanup your git history.

The easy fix is to squash PRs.

The hard fix is to enforce that devs become "crafters" and to define what is and isn't "good faith".

Re: Idiot Proof Git

#346

Post by lamontcg »

evandale wrote: post #248
groestl wrote: post #195

Earlier quoted context omitted.

> 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.

It's a bit heavy handed, but I believe the rules are more like best practices. If you're fixing typos in new code I see no reason to have the history of introducing the typo and then fixing it. If you've come across a typo in a file you're editing then by all means make that typo fix in its own commit. It gets worse when you start fixing typos in files unrelated to your fix. One of those small typo fixes could introd…

> It gets worse when you start fixing typos in files unrelated to your fix.

That does not go into the PR and someone should make you split that out into a code-cleanup PR and it shouldn't pass review.

Re: Idiot Proof Git

#347

Post by Dylan16807 »

P5fRxh5kUvp2th wrote: post #314
idop wrote: post #98

Earlier quoted context omitted.

To be clear, what I'm advocating for is that feature branches get rebased regularly by the developer until PR-time and a clean merge into the mainline. I usually recommend squashing to one commit but do not insist. I can definitely see how those intermediate commits can provide more information, but there's a tradeoff. More often than not, they do not provide me much value, and instead give me bloat, so I prefer to k…

"Why doesn't git bisect work?" "well, it landed on this rebased commit that's huge. I guess it was a kind of useful, just not as useful as we'd like".

But if the alternative is that it's ten commits and most of them don't work anyway, the bisect takes longer to give you the same lousy information.

Re: Idiot Proof Git

#348

Post by Dylan16807 »

chrisfinazzo wrote: post #320
olddustytrail wrote: post #260

Earlier quoted context omitted.

Probably just making them aware of it. But I can give a slightly longer spiel: Commits in git are immutable. They're identified by their hash, so they have to be. What's more they have the hash of the previous commits so the whole chain back to the first commit can't be changed. You can only add new chains. As a consequence, if your main branch points to commit abc123 and your feature branch points to commit def456 t…

I'm certainly aware of reflog and have (thankfully) only had occasion to use it once or twice that I can remember. To my original comment - having to force-push in order to resolve heads - is there a "correct" way to do this that doesn't feel gross?

You're going to have to explain your problem in more detail than "resolve a situation with multiple remote heads".

Re: Idiot Proof Git

#349

Post by Dylan16807 »

mixmastamyk wrote: post #254
ok_dad wrote: post #114

Earlier quoted context omitted.

I rebase on my topic branches because then my edits are neatly stacked on top of the other branch, so I can re-arrange things more easily. Why would I want a weird commit with a bunch of work I didn't do on the topic just smooshed into the middle of my well-crafted series of commits?

Work you didn't do won't be in your branch. There is no rearranging, it is one commit.

> Work you didn't do won't be in your branch.

The merge commit will be in the branch.

> There is no rearranging, it is one commit.

You misread that. They want to be able to rearrange things easily. Having multiple merge commits in the middle gets in the way of that.

Re: Idiot Proof Git

#350

Post by Dylan16807 »

Shacklz wrote: post #151
daneel_w wrote: post #16

I'm incredibly thankful that 99% of my Git usage at work gets away with just PULL, CHECKOUT [-b], COMMIT [--amend] and PUSH. Rarely do I need to rebase, for any reason.

I understand the sentiment, but since git is probably one of the longer-lasting constants in our industry (if not the longest-lasting constant), I personally think it's really worth to have a bit of a look into it. Something I wish someone suggested to me years ago: Instead of trying to understand the commands, try to understand the datamodel. A branch is just a pointer to a commit, a commit is just a pointer (with m…

> Once you understood this (which really isn't any harder than, say, understanding how quick-sort works), going from the data-model to the commands is fairly easy, almost intuitive if it weren't for all the convoluted options that each command can take.

So, not intuitive at all?

I've had to make very minor changes to the commit history that took a bunch of obnoxious commands. I know the git model very well but it didn't help. It would have been easier to copy all the source files out, check out the branch I wanted, then copy them all back in.

Post reply on HN