Comment by david422
5 hours ago
I work with someone who tends to rejects PR suggestions. I also work with someone else who accepts suggestions.
I think that the for the person who accepts suggestions, it's made me wonder if they accept them in part to share ownership with me. I feel like we both maintain and understand the code, and are on the same page.
For the person who rejects PR suggestions, it makes me less inclined to participate in those PRs. Why spend the time doing a thorough review if it's going to get rejected anyways.
Our team tends to prefix all our comments with one of
* thought: Maybe foo'ing is more common in the future - we can refactor if that happens.
* change: This is a leaky abstraction, would prefer to see this modeled like bar instead.
* nit: Naming seems a little unintuitive, consider "Baz", "Boo" maybe?
* fix: This unit test is validating the wrong field.
* chat: This is a big decision and would dictate how solutions of this category look like going forward. Let's bring this to the team first.
----
With the idea that some of those prefixes are stopping the PR until they are changed, and some are just a "take it or leave it" type comments. It makes it unambiguous to the opener that you consider these X things as "We've gotta get on the same page" and these Y things as "Stated preferences" or "just an observation".
word of warning - don't feel bad if you leave a nit, the other person disagrees and ignores it. If you felt strongly about it, it shouldn't have been a nit.
> For the person who rejects PR suggestions, it makes me less inclined to participate in those PRs. Why spend the time doing a thorough review if it's going to get rejected anyways.
This is why you leave blocking suggestions and force the conversation if you think it is important enough.