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

321–330 of 435 posts

Re: Idiot Proof Git

#321

Post by yrgulation »

throwaw20221107 wrote: post #292

Modern Git workflow is very simple on its own: 1. Your codebase has a `main` branch which is write-protected 2. Devs submit changes to `main` from their own branches using PRs 3. Devs can do whatever the fuck they want on their own branch 4. PRs are merged one at a time 5. When merge happens a dev's PR is squashed into one commit that gets appended to `main` 6. If next dev wants to merge their PR with a conflicting c…

Mainly agree except the squashing bit. Squashing means lost history, and you cant tell why a specific method or peace of code was written.

Re: Idiot Proof Git

#322

Post by ratorx »

tremon wrote: post #294
krinchan wrote: post #183

Earlier quoted context omitted.

git add .; and git commit --amend --no-edit; git push origin --force Rarely, I have to do pipeline work on repositories. You'd normally see twenty "Fix Jankins Issue" commits on the main branch because of some nonsense that only happens when you deploy UAT or whatever. Once I learned this little gem, this is also how I manage my feature branches mostly. But also my employer's fleet of laptops has been aging and I've…

git add . We have had multiple security incidents because some developer left a credential file inside the local git clone (no, not all tooling supports out-of-tree stored credentials). Blind 'git add .' is the first thing I teach my developers not to do.

git add . is very useful though. Surely, the first thing to teach here is to always git status before committing?

My typical workflow is to git add . to see the mess I’ve made then decide how to clean it up. If I’ve mistakenly added a credentials file, the fix is to add it to the gitignore and unstage it, not JUST unstage it.

Not saying that you shouldn’t do both, but maintaining a gitignore and completely removing the potential problem for other people seems better than pretending your tool is more limited than it is.

Re: Idiot Proof Git

#323

Post by firesloth »

Zitrax wrote: post #119
wodenokoto wrote: post #63

Pretty cool idea. Something I really want is a version of `git amend` that takes a commit hash, so I can amend my changes to any commit, not just the last one without having to start an interactive rebase.

You could create an alias doing something like a "git commit --fixup " and then a "git rebase --autosquash ~".

I mentioned elsewhere, but I made a simple bash function that does something like that but matches a string in a previous commit message (because I found that easier to type quickly than a commit hash):

  function git-commit-fixup() {
    git commit --fixup ":/$*"
  }
  # usage: suppose there's a commit "fix: the thing"
  git-commit-fixup thing
  # now there's a new commit "fixup! fix: the thing" which can be autosquashed

Re: Idiot Proof Git

#324

Post by ratorx »

throwaw20221107 wrote: post #292

Modern Git workflow is very simple on its own: 1. Your codebase has a `main` branch which is write-protected 2. Devs submit changes to `main` from their own branches using PRs 3. Devs can do whatever the fuck they want on their own branch 4. PRs are merged one at a time 5. When merge happens a dev's PR is squashed into one commit that gets appended to `main` 6. If next dev wants to merge their PR with a conflicting c…

I prefer to not squash, because it loses useful history, but craft the commits in the PR instead.

A small PR might be 1 commit, but splitting the commits for a bigger PR makes it much easier to write good messages for each change. This is useful in future when refactoring and interacts better with e.g. git blame, rather than having 1 big commit that changed files in numerous places.

This is also useful if a PR is broken. It’s easier to revert a simpler commit (if possible) and just fix that, than redo the entire PR with the fix.

Re: Idiot Proof Git

#325

Post by agentwiggles »

jfk13 wrote: post #83
agentwiggles wrote: post #68

Earlier quoted context omitted.

I won't disagree with that, but idk. Once you understand the basic operations everything weird is a Google away. I'll admit that Git is the only VCS I've spent any time with, so maybe I just don't know how much better it could be. But I've almost never had an issue with Git where I actually lost code. And anytime I'm doing something dangerous, I just make a backup copy of the directory in case I screw up irrevocably.…

> almost never ... actually lost code That seems like one of the absolute basics. "Almost" never...? > I just make a backup copy of the directory in case I screw up irrevocably If you really felt you could trust your source control system, that shouldn't be needed, and... > the cases where I've need to use my backup copy ...should be nonexistent.

I guess what I'm trying to express is that even in the rare case I'm doing something fancy with git making sure I don't lose anything is trivial.

I can think of exactly one time I actually lost code and I was doing ill advised reflog fuckery.

I take your point, but I stand by my opinion that in virtually all normal usage, git just isn't that hard. YMMV.

Re: Idiot Proof Git

#326

Post by throwaw20221107 »

yrgulation wrote: post #321
throwaw20221107 wrote: post #292

Modern Git workflow is very simple on its own: 1. Your codebase has a `main` branch which is write-protected 2. Devs submit changes to `main` from their own branches using PRs 3. Devs can do whatever the fuck they want on their own branch 4. PRs are merged one at a time 5. When merge happens a dev's PR is squashed into one commit that gets appended to `main` 6. If next dev wants to merge their PR with a conflicting c…

Mainly agree except the squashing bit. Squashing means lost history, and you cant tell why a specific method or peace of code was written.

I'd argue "why a method exists" should be addressed with naming and javadocs, not in the commit message. Why split the meaning of the code between the code itself and the commit messages?

And if it's not possible to document inline, the PR docs or code review comments should address this. Then future onlookers can use `blame` to see the context.

Re: Idiot Proof Git

#327

Post by throwaw20221107 »

ratorx wrote: post #324
throwaw20221107 wrote: post #292

Modern Git workflow is very simple on its own: 1. Your codebase has a `main` branch which is write-protected 2. Devs submit changes to `main` from their own branches using PRs 3. Devs can do whatever the fuck they want on their own branch 4. PRs are merged one at a time 5. When merge happens a dev's PR is squashed into one commit that gets appended to `main` 6. If next dev wants to merge their PR with a conflicting c…

I prefer to not squash, because it loses useful history, but craft the commits in the PR instead. A small PR might be 1 commit, but splitting the commits for a bigger PR makes it much easier to write good messages for each change. This is useful in future when refactoring and interacts better with e.g. git blame, rather than having 1 big commit that changed files in numerous places. This is also useful if a PR is bro…

>if a PR is broken, revert a simpler commit and just fix that

I guess it differs between "library development" and "service development".

When you're developing a service, what's in `main` is constantly being tested under the production/customer load. So reverting the known bad PR is a faster fix. And the bug is affecting your customers, so you fix fast and cleanup later.

Typically in industry if a PR is broken you revert the whole thing as fast as possible. But most projects in industry are services.

However when you're developing a library you'll probably release via tags. So if bad code makes it into `main`, it's not a huge deal, and it might be clearer to just revert the one (child) commit of the PR. Because there's no time crunch, no bug in prod, nothing affecting customers.

>but craft the commits in the PR instead.

The problem with not enforcing squashing is trust. Think about the worst dev on your team:

Given that trust, are they gonna split their PR into useful commits? Fuck no. they're gonna merge into `main` a bunch of commits saying "fix typo lol xd" and "lol I suck".

If you give them the power to not just make 1 commit per PR, but N commits per PR, are they going to fill up your team's history with crap? Absolutely they will.

Squashing is a useful gatekeep in that way, to prevent one person from screwing over everybody else and making it harder to `blame` to rootcause a critical outage.

Re: Idiot Proof Git

#328

Post by eru »

wackget wrote: post #110
eru wrote: post #59

Earlier quoted context omitted.

So there's (at least) two uses for commits. The first is to keep a log of what you are working on. For me, that's lots of small and dumb commits. The second is to provide a story for review. Most of the time, when the change you are putting out for review is small and simple, you can just put it all into a single commit. Sometimes, your change is more complicated and it makes sense to break it into a series of relate…

Why wouldn't you use a local branch (or something?) for your ugly commits and then merge everything from that branch into the main shared branch when you're done?

Because that's hard to review.

I am doing a lot of exploratory programming and way point commits.

I don't need the reviewer to understand all the mistakes I made and bad designs I considered. It's enough work to understand the finished design.

As a rule of thumb, every commit that lands in master's history should build and pass tests. So that eg 'git bisect' works.

But it's not a good idea to put that same requirement on waypoint commits I make along the way, when exploring.

Re: Idiot Proof Git

#329

Post by eru »

__blockcipher__ wrote: post #233
wackget wrote: post #110

Earlier quoted context omitted.

Why wouldn't you use a local branch (or something?) for your ugly commits and then merge everything from that branch into the main shared branch when you're done?

Because then you're going to merge in the ugly commits and make everyone who needs to look at the history in the future have to work that much harder to understand what's going on.

And you aren't just making the human's job harder, but also tools like 'git bisect' work better when every commit in master builds and passes tests.

Re: Idiot Proof Git

#330

Post by eru »

mixmastamyk wrote: post #255
aulin wrote: post #124

Earlier quoted context omitted.

if you're the only one working on that branch I don't see where is the problem in rewriting history and force pushing

Until you make a mistake.

Why? That's what 'git reflog' is for.

Or you mean that a mistake where you accidentally push to someone else's branch?

The default model that public github uses is good for that: everyone works on their own fork of the repo, and makes pull requests to the shared repo. Nobody pushes directly to the shared repo.

Post reply on HN