logoalt Hacker News

skydhashyesterday at 6:56 PM2 repliesview on HN

IMO, in a team settings, improving the review policies and speed has a much better benefit. A PR is supposed to be a proposal for some change, adding more proposals on top of something that is not reviewed is a bit icky.

> . By focusing the stack to the different reviewers you can avoid ambiguity about "what a person is signing off on" in the stack.

That can be easily done with comments. If the PR are orthogonal, they could have been split. And if they're not, I would really like to know how the part that I'm reviewing interacts with the rest of the changes.


Replies

a_t48yesterday at 7:12 PM

The PRs may be orthogonal but still be dependent. Feature X depends on improvement Y which also needs bugfix Z. You might go and implement X in a branch, tweaking the codebase as you go, but split the branch apart for review. You put X/Y/Z up, but X contains Y and Z, which means you can't request reviews for X without Y and Z merging, or else have a bunch of extra code that gets in the way.

show 1 reply
dastbeyesterday at 7:49 PM

> That can be easily done with comments. If the PR are orthogonal, they could have been split.

Comments are ad-hoc and don't scale, relying on the author to interpret and adhere to the extent of the reviewers approval.

> And if they're not, I would really like to know how the part that I'm reviewing interacts with the rest of the changes.

you are free to look up, down, and around the stack; nobody is hiding the code from you. But in many cases this is just unnecessary.