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

Git commands I run before reading any code

piechowski.io

Post reply on HN
Print view View original

61–70 of 546 posts

Re: Git commands I run before reading any code

#61

Post by Aachen »

seba_dos1 wrote: post #42
Aachen wrote: post #21

Earlier quoted context omitted.

If someone uses git commits like the save function of their editor and doesn't write messages intended for reading by anyone else, it makes sense to want to hide them For other cases, you lose the information about why things are this way. It's too verbose to //comment on every like with how it came to be this way but on (non-rare in total, but rare per line) occasion it's useful to see what the change was that made…

> If someone uses git commits like the save function of their editor I use it like that too and yet the reviewers don't get to see these commits. Git has very powerful tools for manipulating the commit graph that many people just don't bother to learn. Imagine if I sent a patchset to the Linux Kernel Mailing List containing such "fix typo", "please work now", "wtf" patches - my shamelessness has its limits!

Seems like a lot of extra effort (save, add, commit, come up with some message even if it's a prayer to work now) only to undo it again later and create a patch or alternate history out of the final version. Why bother with the intermediate commits if you're not planning for it to be part of the history?

Re: Git commands I run before reading any code

#62

Post by theshrike79 »

tasuki wrote: post #52
theshrike79 wrote: post #11

Earlier quoted context omitted.

Can you explain to me (an avid squash-merger) what extra information do you gain by having commits that say "argh, let's see if this works", "crap, the CI is failing again, small fix to see if it works", "pushing before leaving for vacation" in the main git history? With a squash merge one PR is one commit, simple, clean and easy to roll back or cherry-pick to another branch.

You gain the extra information by having reasonable commit messages rather than the ones you mentioned. To fix CI you force push. Can you explain to me what an avid squash-merger puts into the commit message of the squashed commit composed of commits "argh, let's see if this works", "crap, the CI is failing again, small fix to see if it works", and "pushing before leaving for vacation" ?

The squashed commit from the PR -> main will have a clean title + description that says what was added.

Usually pretty close to what the PR title + description are actually, just without the videos and screenshots.

Example:

feat(ui): Add support for tagging users

* Users can be tagged via the user page * User tags visible in search results (configurable)

etc..

I don't need to spend extra time cleaning up my git commits and force-pushing on the PR branch, losing context for code reviews etc. Nor does anyone have to see my shitty angry commits when I tried to figure out why Playwright tests ran on my machine and failed in the CI for 10 commits.

Re: Git commands I run before reading any code

#63

Post by Pay08 »

jbjbjbjb wrote: post #53
ramon156 wrote: post #5

> The 20 most-changed files in the last year. The file at the top is almost always the one people warn me about. “Oh yeah, that file. Everyone’s afraid to touch it.” The most changed file is the one people are afraid of touching?

This command needs a warning. Using this command and drawing too many conclusions from it, especially if you’re new, will make you look stupid in front of your team mates. I ran this on the repo I have open and after I filtered out the non code files it really can only tell me which features we worked on in the last year. It says more about how we decided to split up the features into increments than anything to do w…

Good thing that the article contains that warning, then.

Re: Git commands I run before reading any code

#64

Post by mcpherrinm »

seba_dos1 wrote: post #8

> If the team squashes every PR into a single commit, this output reflects who merged, not who wrote. Squash-merge workflows are stupid (you lose information without gaining anything in return as it was easily filterable at retrieval anyway) and only useful as a workaround for people not knowing how to use git, but git stores the author and committer names separately, so it doesn't matter who merged, but rather wheth…

Squash merge is the only reasonable way to use GitHub:

If you update a PR with review feedback, you shouldn’t change existing commits because GitHub’s tools for showing you what has changed since your last review assume you are pushing new commits.

But then you don’t want those multiple commits addressing PR feedback to merge as they’re noise.

So sure, there’s workflows with Git that doesn’t need squashing. But they’re incompatible with GitHub, which is at least where I keep my code today.

Is it perfect? No. But neither is git, and I live in the world I am given.

Re: Git commands I run before reading any code

#65

Post by theshrike79 »

seba_dos1 wrote: post #18
theshrike79 wrote: post #11

Earlier quoted context omitted.

Can you explain to me (an avid squash-merger) what extra information do you gain by having commits that say "argh, let's see if this works", "crap, the CI is failing again, small fix to see if it works", "pushing before leaving for vacation" in the main git history? With a squash merge one PR is one commit, simple, clean and easy to roll back or cherry-pick to another branch.

These commits reaching the reviewer are a sign of either not knowing how to use git or not respecting their time. You clean things up and split into logical chunks when you get ready to push into a shared place.

Why would the reviewer look at the commit messages instead of the code?

1. Open PR page in whatever tool you're using

2. Read title + description to see what's up

3. Swap to diff and start reading through the changes

4. Comment and/or approve

I've never heard anyone bothering to read the previous commit messages for a second, why would they care?

Re: Git commands I run before reading any code

#66

Post by arccy »

palata wrote: post #39
Jenk wrote: post #30

Earlier quoted context omitted.

Tbf you wouldn't use/switch to jj for (because of) those kind of commands, and are quite the outlier in the grand list of reasons to use jj. However the option to use the revset language in that manner is a high-ranking reason to use jj in my opinion. The most frequent "complex" command I use is to find commits in my name that are unsigned, and then sign them (this is owing to my workflow with agents that commit on m…

Actually, signing was one of the annoying parts of jujutsu for me: I sign with a security key, and the way jujutsu handled signing was very painful to me (I know it can be configured and I tried a few different ways, but it felt inherent to how jujutsu handles commits (revisions?)).

The only reasonable way to use signing in jj is with the sign-on-push config https://docs.jj-vcs.dev/latest/config/#automatically-signing... rather than as commits are made

Re: Git commands I run before reading any code

#67

Post by lamasery »

seba_dos1 wrote: post #8

> If the team squashes every PR into a single commit, this output reflects who merged, not who wrote. Squash-merge workflows are stupid (you lose information without gaining anything in return as it was easily filterable at retrieval anyway) and only useful as a workaround for people not knowing how to use git, but git stores the author and committer names separately, so it doesn't matter who merged, but rather wheth…

Squash-merge is entirely fine for small PRs. Cleaning up the commits in advance (probably to just squash them to one or two anyway) is extra work, and anything that discourages people from pushing often (to get the code off their local machine) needs to be well-justified. Just review the (smallish!) total outcome of all the commits and squash after review. A few well-placed messages on the commit, attached to relevant lines, are more helpful and less work than cleaning up the commit history of a smallish PR.

For really large PRs, I’m more inclined to agree with you, but those should probably have their own small-PR-and-squash-merge flow that naturally cleans up their git history, anyway.

I categorically disagree that squash-merge is “stupid” but agree there are many ways to skin this cat.

Re: Git commands I run before reading any code

#68

Post by theshrike79 »

tasuki wrote: post #49
yokoprime wrote: post #27

Earlier quoted context omitted.

Haha, good luck working with a team with more than 2 people. A good reviewer looks at the end-state and does not care about individual commits. If im curious about a specific change i just look at the blame.

> A good reviewer looks at the end-state and does not care about individual commits. Then I must be a bad reviewer. In a past job, I had a colleague who meticulously crafted his commits - his PRs were a joy to review because I could go commit by commit in logical chunks, rather than wading through a single 3k line diff. I tried to do the same for him and hope I succeeded.

And then someone comments on a thing, they change it and force-push another "clean" history on top and all of your work is wasted because the PR is now completely different =)

Re: Git commands I run before reading any code

#69

Post by theshrike79 »

seba_dos1 wrote: post #38
yokoprime wrote: post #27

Earlier quoted context omitted.

Haha, good luck working with a team with more than 2 people. A good reviewer looks at the end-state and does not care about individual commits. If im curious about a specific change i just look at the blame.

I have no troubles working on big FLOSS projects where reviews usually happen at the commit level :)

So if a PR consists of 20 commits, they review every single commit linearly without looking at the end result first?
Post reply on HN