Hacker News Reader

Comment by lozenge | Hacker News Reader

Not really. The idea is to split work into separate stages which are reviewed separately, but as a whole.

In the example: "small refactor 25LOC -> new API 500LOC -> migrate API users 50LOC"

Making a PR of the small refactor will probably garner comments about "why is this necessary".

Opening two PRs at the same time is clutter as GitHub presents them as separate.

As well, sometimes CI won't pass on one of the stages meaning it can't be a separate PR, but it would still be useful in the code review to see it as a separate stage.

View on HN

I'd be quite happy with seeing the three jobs in the article as three separate PRs. Fixing a bug and adding a feature are two jobs that, as I think we all agree, need to be tracked individually - so work on them individually.

> As well, sometimes CI won't pass on one of the stages meaning it can't be a separate PR

Could you give an example of this? Not sure what you mean.

Commits aren't always perfect.

Sometimes I'll make the unit test first, which fails CI and the next set of commits implements the behavior.

By doing this, you break commit atomicity and make bisects hell. Please don’t do this. Commits aren’t perfect at first for sure, but they should be by the time you make them reviewable.

It's fine to break commit atomicity on feature branches. You can use git bisect --first-parent on you development/master branch.

I completely disagree. In doing so you lose all visibility into the components and gradual evolution of the code that atomic commits provide. Same thing with squashing (which is just the worst).

the comments about "why is this necessary" can be handled with a decent PR template, and a comment.

What I tend to do is make the changes locally with different commits and then cherry pick the refactor into a PR branch and wait for that to be accepted. Then I rebase the FULL branch with "master" after the merge and create the PR.

Reply on news.ycombinator.com