Hacker News new | ask | show | jobs
by xorcist 15 days ago
> if you squash the commits when merging the PR?

You tidy up and rebase before making a PR. Anything else is really disrespectful of your reviewer's time. That is also how all the larger open source projects operate.

> you only get one commit for the whole feature

If you are doing one logical commit per PR, you are doing way too many PRs.

Alternatively you don't have a working review process.
2 comments
account42 14 days ago
Yes, often you have separate logical changes that should still be reviewed together.
link
skydhash 14 days ago
Not really, because that one commit represent a logical changes to the codebase. It’s either in or not. Splitting it would be only cosmetic. That’s what a good PR in my opinion.

Presenting a series of patches is good in an email format because when I’m adding them, I can evaluate each and decide whether I want it or not. But GitHub (and forges that copies it) is lacking in that regards without me taking over the branch.

So the word is to make the PR the unit of changes, and only review the whole diff, not the individual commit.

link