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

Commit often, perfect later, publish once: Git best practices (2013)

sethrobertson.github.io

Post reply on HN
Print view View original

71–80 of 117 posts

Re: Commit often, perfect later, publish once: Git best practices (2013)

#71

Post by alecbz »

cjfd wrote: post #4

I kind of feel that this kind of git advice is way beyond the point of diminishing returns. As a conscientious developer we have a lot of work. We write code of good quality. We refactor that code regularly. We write automated tests. We test the program manually. We use linters and type checkers. We talk to people to find out whether what they requested is actually what they need. But the day only has 24 hours. At so…

You should only ever do things when they're worth it. Which sounds obvious, but a lot of people argue for "correctness" of things in the abstract without tying back to why that correctness matters, how much its worth, and what the cost of getting it is.

In the case of git, I think

a) a good "published" code history can actually be pretty valuable

b) git's UX is bad

The fact that a tidy history is valuable should be totally independent from the fact that git's UI doesn't make it easy to do that.

Re: Commit often, perfect later, publish once: Git best practices (2013)

#72

Post by lamontcg »

> Once you git push (or in theory someone pulls from your repo, but people who pull from a working repo often deserve what they get) your changes to the authoritative upstream repository or otherwise make the commits or tags publicly visible, you should ideally consider those commits etched in diamond for all eternity.

I've broken this rule multiple times per day for the past 10 years.

On your own feature branches, rebase your fucking shit and force push. I see so many people creating ungodly messes because they never want to erase the history of PRs that they've submitted and its just a nightmare of merge commits pulled back into their branch from master.

I've watched a decade of git n00bs practice this "never under any circumstances rewrite history" advice and it fucks them up over and over and over again.

Nobody actually cares about the exactly commit process you went through to fix the bug. Squash everything and rebase. Leave a SUMMARY of why you did what you did in the PR and/or commit message. Humans have this amazing ability to write stories about what they did after the fact.

I'll frequently leave my-future-self notes on closed and merged PRs as I think about them post-merge, where I document what approaches were rejected, and what approaches might be worthwhile if the change isn't sufficient. Stuff like "I could not do WWWW because of , so instead we must do XXXX, if this is not sufficient because then we must consider that YYYYY will be less preferable due to and we should consider doing ZZZZZ first". If all you do is capture "what did I change" and don't capture your thinking and what you view to be all the different alternatives while your mind is still fresh with the problem then you're just throwing away useful information, which is what the "preserve your git history" approach does.

In the future, when I read my PRs I just don't care about how I got there. I care about what I was thinking about. So I write down, long-form, what I was thinking about. A git history is about going from A to B, it might document why you bailed on going to C, but it probably doesn't capture the stuff you rejected the whole time like D E and F and why. And really I'd PREFER to read a good note on why C sucked as a solution. I don't need to see the code that went down the route of C until it turned into a mess and then had to be backed out.

Re: Commit often, perfect later, publish once: Git best practices (2013)

#73

Post by alecbz »

danny_sf45 wrote: post #45
seba_dos1 wrote: post #24

Earlier quoted context omitted.

Isn't the answer to "is this worth it" quite obvious when doing code reviews? Properly and logically split commits can make it so much easier and more pleasant, and things like "commit often, perfect later" are in my experience less, not more work. The rest seems like a pretty basic set of tips that I mostly learned over the years because I needed them, so it would be definitely nice to read such article several year…

Do people actually do code reviews per commit? I know it's a thing, but in all my years of experience, I haven't met anyone who actually does this. The usual practice is to review all the changes at once (e.g., go to the "Files changed" tab of a PR in GitHub and start reviewing the changes). This, of course, means, that PR are "small". If a PR is too "big" then one politely asks the author to split the PR in many.

> Do people actually do code reviews per commit?

This is sort of a meaningless question without an understanding of how often people commit. Some people keep a single commit for a single PR (and just constantly update that one commit), others make tiny little commits for every individual change.

The only real question is: what is the granularity you should review code changes at? The number of git commits that that maps to doesn't really matter.

IMO: the ideal is to try to keep pull requests as small as possible while still having each PR be a coherent, justifiable change on its own. I don't know if it's realistic to treat that as a hard rule, but I think it's the right thing to aim for.

Re: Commit often, perfect later, publish once: Git best practices (2013)

#74

Post by alecbz »

jdowner wrote: post #27

I am beyond sick of hearing about 'best practices' for everything. It is such an obnoxious statement! So often it is used as replacement for 'in my opinion' because it brooks no argument -- these are the BEST practices!

I can't stand when I ask someone "why?" and they answer "it's a best practice". Yeah, I'm asking _why_ you think this is a best practice.

Re: Commit often, perfect later, publish once: Git best practices (2013)

#75

Post by simonw »

cjfd wrote: post #4

I kind of feel that this kind of git advice is way beyond the point of diminishing returns. As a conscientious developer we have a lot of work. We write code of good quality. We refactor that code regularly. We write automated tests. We test the program manually. We use linters and type checkers. We talk to people to find out whether what they requested is actually what they need. But the day only has 24 hours. At so…

I've realized that as software engineers our work isn't writing code: it's changing code.

As such, the unit of work that we produce is the code change.

Which means crafting a good commit - with a good commit message - is key to our craft.

Re: Commit often, perfect later, publish once: Git best practices (2013)

#76

Post by angrais »

alecbz wrote: post #73
danny_sf45 wrote: post #45

Earlier quoted context omitted.

Do people actually do code reviews per commit? I know it's a thing, but in all my years of experience, I haven't met anyone who actually does this. The usual practice is to review all the changes at once (e.g., go to the "Files changed" tab of a PR in GitHub and start reviewing the changes). This, of course, means, that PR are "small". If a PR is too "big" then one politely asks the author to split the PR in many.

> Do people actually do code reviews per commit? This is sort of a meaningless question without an understanding of how often people commit. Some people keep a single commit for a single PR (and just constantly update that one commit), others make tiny little commits for every individual change. The only real question is: what is the granularity you should review code changes at? The number of git commits that that m…

Yes, but the OP asked if people are reading specific commits in a PR.

The answer for me at last is no. Because the history in a PR should not matter but only the final commit. That's why we choose to squash commits in my current role.

Re: Commit often, perfect later, publish once: Git best practices (2013)

#77

Post by angrais »

vlovich123 wrote: post #51
danny_sf45 wrote: post #45

Earlier quoted context omitted.

Do people actually do code reviews per commit? I know it's a thing, but in all my years of experience, I haven't met anyone who actually does this. The usual practice is to review all the changes at once (e.g., go to the "Files changed" tab of a PR in GitHub and start reviewing the changes). This, of course, means, that PR are "small". If a PR is too "big" then one politely asks the author to split the PR in many.

When I was at FB, there’s no PR as defined at GitHub. Each independent commit was a separate review. So in that sense, all of the 10s of thousands of engineers at Facebook daily review “PRs” at the commit level.

In that case, where you then recommended to only make one commit? Are there guidelines for the average/ideal commit length? Did this add to extra workload in creating "perfect" commits? Likewise, did this mean that pushing "WIP" commits to remote was rare?

Re: Commit often, perfect later, publish once: Git best practices (2013)

#78

Post by jollybean »

jrockway wrote: post #13
cjfd wrote: post #4

I kind of feel that this kind of git advice is way beyond the point of diminishing returns. As a conscientious developer we have a lot of work. We write code of good quality. We refactor that code regularly. We write automated tests. We test the program manually. We use linters and type checkers. We talk to people to find out whether what they requested is actually what they need. But the day only has 24 hours. At so…

It really depends on the timescale of your changes. Change a few lines in an afternoon? Nobody cares about the history. But if you ever have a long-running branch, you will care about your local history. You'll merge in the main branch, and it will conflict. Someone already renamed the thing that you're renaming in your branch. An auto-formatted changed its mind. The API changed. Some stuff was refactored. The differ…

" Change a few lines in an afternoon? Nobody cares about the history."

If you changed a couple lines of code as part of a module refactor or something i.e. ongoing changes, then sure.

But if you were fixing a bug or doing some very specific thing, those are actually the times where hyper-specific contextual commits are necessary.

Git is this generic tool for which we still have not yet learned all the best practices and idiomatic usages.

Probably there should be a handful of types of commits, not more than 3-5 i.e. 'change' 'bug fix' 'upgrade' 'internal release' 'major release' (these are probably bad examples).

Re: Commit often, perfect later, publish once: Git best practices (2013)

#79

Post by bonzini »

LAC-Tech wrote: post #29
cjfd wrote: post #4

I kind of feel that this kind of git advice is way beyond the point of diminishing returns. As a conscientious developer we have a lot of work. We write code of good quality. We refactor that code regularly. We write automated tests. We test the program manually. We use linters and type checkers. We talk to people to find out whether what they requested is actually what they need. But the day only has 24 hours. At so…

I agree 100%. I wish we could use a version control tool that didn't require so much attention. I don't want to read article after article for something that should just get out of my way.

It's out of your way if you so desire, just squash and commit. What you pay the price for isn't satisfying git's whims; it's a better debugging or learning experience months or years down the line, which is something that git enables you to do.

Re: Commit often, perfect later, publish once: Git best practices (2013)

#80

Post by sime2009 »

danny_sf45 wrote: post #45
seba_dos1 wrote: post #24

Earlier quoted context omitted.

Isn't the answer to "is this worth it" quite obvious when doing code reviews? Properly and logically split commits can make it so much easier and more pleasant, and things like "commit often, perfect later" are in my experience less, not more work. The rest seems like a pretty basic set of tips that I mostly learned over the years because I needed them, so it would be definitely nice to read such article several year…

Do people actually do code reviews per commit? I know it's a thing, but in all my years of experience, I haven't met anyone who actually does this. The usual practice is to review all the changes at once (e.g., go to the "Files changed" tab of a PR in GitHub and start reviewing the changes). This, of course, means, that PR are "small". If a PR is too "big" then one politely asks the author to split the PR in many.

Not really. PRs have become the new commits. I only pay attention to the changed files tab in GH. I encourage other people to commit early and often and share their progress with the rest of the team.
Post reply on HN