Hacker News new | ask | show | jobs
by OJFord 639 days ago
If it's large I want to be able to look at the commits in isolation and understand the work in the logical chunks that made sense to the author.

If what made sense to them is temp, checkpoint, temp, temp, undo the temp, fix test, try this, that didn't work try other thing, maybe?, temp, fix test - then I don't stand a chance. Recently I reviewed one that had multiple 'rebase' commits, I have no idea.
1 comments
scott_w 639 days ago
> If it's large

Then make the PR small.

link
keybored 638 days ago
This is a conversation that keep repeating

- Many temp commits

- Doesn’t matter: just squash

- But sometimes I want to have a few distinct (isolated commits for my PR)

- Then make the PR small

- But I have several changes

- Then make several small PRs

We keep coming back to “just make more PRs”. Which is curious, given that GitHub doesn’t even support dependent PRs. The thing you need immediately when your PRs start depending on each other (like refactor X before implementing Y, where both touch the same code).

But I can’t come to any other conclusion than that this is because of the over-focus on PR as the one and only reviewable unit. Thanks to GitHub.

I can’t really get on the small PR train. A PR has overhead and I often want to do small changes that I only happen to notice are necessary when I am in the middle of doing that one PR.

link
scott_w 638 days ago
Don’t make dependent PRs. Make small PRs, review, merge and deploy them before starting the next.
link
keybored 638 days ago
That begs other questions. The relevant reviewer isn’t available until Wednesday. Do I start on the next tasks in dependent branches? The act of making all those branches is a large part of the inherent busywork. And then I need to remember to get back to them when the reviewer gets back.

The benefit of all these small PRs is yet to be revealed.

link
scott_w 638 days ago
Why are you starting work you can’t finish for days/weeks?
link
keybored 638 days ago
Is the dialogue here that you ask questions that makes the workflow more and more constrained and then when X factors are fixed it becomes obvious that the workflow works great?

Of course I start on work that I (not the reviewer) can work on right now.

link
OJFord 639 days ago
1. I'm reading not writing.

2. Is our 'large' the same?

3. I'd rather the PR were whatever size it needs to be to entirely do the thing it's supposed to do (and nothing else) than conform to some arbitrary size requirement.

link
scott_w 638 days ago
You're over-thinking it. Large PRs have a known effect of reviewers' eyes just glaze over and the PR just gets a cursory glance and LGTM :+1: on it.

That's when you know a PR is "large."

What's stopping you breaking such a PR into smaller chunks? Some arbitrary "it does what it's supposed to do" definition?

link