4 ms·
The fact that your comment is currently at the top of the thread tells me that a lot of people go through this, yet this is interesting to me because past HN di
by ravenstine 3y ago
The fact that your comment is currently at the top of the thread tells me that a lot of people go through this, yet this is interesting to me because past HN discussions have suggested said experience to be an exception. In my own experience, blocking (or simply not approving) PRs over nits, out-of-scope improvements, and mere personal preferences is the norm. What's worse is when PRs get blocked because a person doesn't like the pattern the author is using even though it's already one of many existing patterns and there's no explicit guideline anywhere. I find this happens a lot when lead and staff engineers make decisions on what's "considered harmful" but don't actually broadcast that decision in an effective way.
What I try to do with my code reviews is prefix my comments with "Non-blocking suggestion:" or "Minor question:" so that the other person is more likely to know that a particular piece of feedback doesn't come with the expectation of needing to be addressed.
Otherwise, if the code works, has tests, and isn't objectively incorrect, I try to give an approval. If you don't like something about it, there should be room in the process to make improvements. The idea that "mistakes won't actually get fixed later" is a cop out. If your team can't fix mistakes, then you should turn in your "engineer" titles.
Also, when programmers do things that others on their team disagree with, it almost always comes down to a breakdown in communication. Junior developers are a bit of a different story, but senior developers not knowing how to write code that the rest of the team approves of means that people aren't communicating, and it probably means that your system is so complex that it's unreasonably difficult to determine the proper approach in the first place.