Remix.run Logo
jxf 2 hours ago

> No one is ever going back and reading individual commits.

I violently disagree with this.

At a minimum, when I review PRs I look at the commit history to understand what's up. If the path that was taken to commit this is full of "oops" and "fix" messages, it's an immediate reject for me. The commits tell the story and it's a kindness to your human reviewers to not make them work harder to understand the point you're trying to get across.

GrandfatherTECH 2 hours ago | parent | next [-]

What's up with the fix commits? Maybe I misunderstood you, but there ain't nothing wrong in fixing stuff you offer in your PR. And there can also be multiple commits even before the PR while you're developing your PR.

locknitpicker an hour ago | parent [-]

> What's up with the fix commits?

They shouldn't show up in the commit history. In a PR, you merge them in the commit that they actually fix. Otherwise when you use git blame to get the context of why a line of code was changed, all you see is a useless "fixup" message that is worse than having nothing.

Anyone can do better than a fixup commit. And doing metter means merging them into the actual commits that are fixed.

nl 2 hours ago | parent | prev | next [-]

> If the path that was taken to commit this is full of "oops" and "fix" messages

great way to encourage people to rebase then!

fcarraldo 2 hours ago | parent | prev | next [-]

Mine aren’t full of “oops” and “fix” messages, because I squashed them.

JohnMakin an hour ago | parent | prev | next [-]

yea I look at commits several times a week at least, especially when commits are tied to a ticketing system/project it helps a lot going back months later on a large codebase going “how/why did this change happen”

I do tend to squash or make my entire change in one commit though so maybe I misunderstood your comment. If I have a fix commit often I’ll just tag a separate PR/ticket to keep the change history/change control clean

InvertedRhodium 2 hours ago | parent | prev | next [-]

Depends what the git history is supposed to show. Personally, I prefer people to leave their mistakes and reversions - though I'd require more description messages than "oops" or "fix", something that explained why it was being reverted or swapped out would be the minimum.

Sometimes you try things one way and they don't work out, so you go in a different direction. Capturing why this happened and when can go a long way towards explaining downstream decisions that might seem confusing to someone with a fresh perspective.

gofreddygo an hour ago | parent | prev | next [-]

Oh that is such a bad heuristic ! The commits and history of how a PR was put together is no indicator of the quality of the PR or the thought process that led to it. Thats the equivalent of rejecting a (handwritten) essay for having too many corrections. ridiculous.

The code is all that should matter. Maybe comments for being nice to others and my future self. thats it.

locknitpicker an hour ago | parent [-]

> The commits and history of how a PR was put together is no indicator of the quality of the PR or the thought process that led to it.

It is, because it means the person posting the PR didn't even bothered to review the changes they are forcing others to review.

Just clean after yourself before asking others to read your stuff.

bobbylarrybobby 2 hours ago | parent | prev | next [-]

Do you really care if someone forgot to format before committing? They can always squash and push locally if they need to.

locknitpicker an hour ago | parent [-]

> Do you really care if someone forgot to format before committing?

Not OP but yes I definitely do. If you expect others to spend time reviewing your code, you are obligated to start off by reviewing it yourself. Posting a mess helps no one and makes code harder to audit.

danieltanfh95 2 hours ago | parent | prev [-]

NGL AI usage is driven by friction in presentation and communication over petty details.