3 ms·
>I assume the way you wrote it is that you already had the real version ready so you knew the API was sufficient even when you only sent the null version for re
by munksbeer 3y ago
>I assume the way you wrote it is that you already had the real version ready so you knew the API was sufficient even when you only sent the null version for review.
Yes. But not only that, just wiring in the null implementation required several choices. I wanted to know that reviewers would focus on that important detail and it not get drowned out by a review of business logic.
Larger PRs often have large parts of the code rubber stamped through sheer wariness of the reviewers. Like it or not, that happens. And that way design choices are approved that should not have. I see it all the time.
You can argue that is the fault of the review system, but if it just constantly happens, how do you fix it? Keep banging at people to review properly? They're people, they're goin to take a path of least resistance, that just happens. If it were up to me I'd put in a CI failure step for PRs over 500 lines. You could override that step if people deemed it reasonable (eg. large search and replace PRs), but it would have to be used judiciously.
I just generally find people who raise large PRs don't respect the reviewer enough to make the effort. So I often leave those PRs sitting, but I know someone is just going to come along and rubber stamp them.