(no title)
rokkamokka | 15 days ago
So, I'm kind of okay with large PRs as long as they're one logical unit. Or, maybe rather, if it would be less painful to review as one PR rather than several.
rokkamokka | 15 days ago
So, I'm kind of okay with large PRs as long as they're one logical unit. Or, maybe rather, if it would be less painful to review as one PR rather than several.
captainbland|13 days ago
alan-stark|13 days ago
strken|13 days ago
nh2|13 days ago
Splitting "handling the happy vs error path" sounds even worse. Now I first have to review something that's obviously wrong (lack of error handling). That would commit code that is just wrong.
What is next, separating the idea from making it typecheck?
One should split commits into the minimum size that makes sense, not smaller.
"Makes sense" should be "passes tests, is useful for git bisect" etc, not "has less lines than arbitrary number I personally like to review" - use a proper review tool to help with long reviews.
sublinear|13 days ago
I still disagree. Why was the feature not split up into more low-level details? I don't trust that kind of project management to really know what it's doing either.
I am not promoting micromanagement, but any large code review means the dev is having to make a lot of independent decisions. These may be the right decisions, but there's still a lack of communication happening.
Hands off management can be good for creativity and team trust, but ultimately still bad for the outcome. I'm speaking from my own experience here. I would never go back to working somewhere not very collaborative.
cik|13 days ago
But, it becomes incumbent on the author to write a guide for reviewing the request, to call the reviewer's attention to areas of interest, perhaps even to outline decisions made.
sublinear|13 days ago
I'm not saying that doesn't work, but writing a guide means the author is now also doing all the planning too.
A successful "guide" then becomes more about convincing the reviewer. The outcome is either a lot of friction, or the reviewer is just going through the motions and trust is eroding.
dboreham|13 days ago
fastasucan|13 days ago