How to deal with a boss who is very nitpicky in code reviews?
My direct supervisor/ tech lead has a tendency to leave a lot of nitpick comments on my and my teammate's PRs during code reviews. I know the intention is positive, but in my view it's excessive and leads our team to spend too much time addressing stylistic or minor changes that don't materially improve the codebase. Since we're building for an early-stage startup I also believe it's a higher priority to ship code that works well enough so the business can get customer feedback, rather than focus on subjective stylistic things. It also increases the noise level in every PR and makes it hard to identify and focus on any comments about significant things.
I raised these concerns directly with my supervisor and also asked for clarification about which nitpicks are actually optional or if I can opt out of implementing any. My supervisor said I need to address/respond to every single nitpick comment, which means if I disagree or don't want to implement the nit, I would have to explain why every single time, which I think is not an efficient use of time. They also said we don't have to implement everything they suggest, and they welcome pushback, but I don't think they realize it feels a bit harder to do that when they leave so many nits and they are in a position of authority and are not my peer. I shared a suggestion that we make it ok for the PR owner to opt out of addressing a nitpick / leave it up to them to decide, and also try not to focus continually on stylistic things that can't be automated by linting etc. This was ignored though.
Does anyone have any advice on how to handle this situation? It's very frustrating and exhausting sometimes, and part of me has tended to cave in and just implement every bit unless I have a really strong opinion against it for the sake of avoiding spending time debating too much. I'd like to be wise about picking my battles.
![Mid-Level Software Engineer [SDE 2] at Amazon company logo](/_next/image/?url=https%3A%2F%2Ffirebasestorage.googleapis.com%2Fv0%2Fb%2Ftech-career-growth.appspot.com%2Fo%2Fcompany_logos%252Famazon_logo.jpg%3Falt%3Dmedia%26token%3Dff5f2302-08be-43bc-b7b1-d381ba508d99&w=48&q=75){kind=logo}
Discussion
(6 comments)- 29 months agoEng @ Taro
but in my view it's excessive and leads our team to spend too much time addressing stylistic or minor changes that don't materially improve the codebase
This is a situation where I would take the time to add a linter as part of the CI process. There is an upfront cost, of course, but the time spent to set it up will help the entire team save time down the road. The current cost isn't just fixing stylistic errors, but the cost is the turnaround time it takes for the TL to read the code, write out the comments, and for you to address the comments.
I recommend writing out a doc with the recommended rules and invite the TL to contribute to the doc, too. The goal of this is to establish a set of rules that you, your teammates, and your TL can agree on that will save everyone time in the future. it's also an excellent project to boast about in your next performance review.
- 0
9 months agoSoftware Engineer [OP]Early-stage startupThanks for taking time to share your suggestion! I like the idea of writing out a doc with suggested rules as something for everyone to discuss as a jumping off point.
- 1
9 months agoTech Lead @ Robinhood, Meta, Course HeroLinters are surprisingly hard to set up (even at Robinhood, it was fairly janky), so I recommend against doing a linter. Here are some lighter alternatives:
- Style doc - This one is the easiest and what I recommend for an early-stage startup. Once you have a doc with all the common nitpicks, every engineer can just run through the doc manually as a checklist before they submit a PR.
- Common IDE style hotkey - This one is trickier, but we did it at Robinhood. It's a nice middle-ground between the primitive style doc and the heavy linter solution. Every IDE should have an auto-format hotkey, and you can program in the settings what formatting it applies (i.e. how many spaces per tab, should brackets go after or before the conditional, etc). After you do this, you should be able to export the style config and have everyone import it into their IDEs. You can host the config file somewhere and bake it into the onboarding wiki.
- 2
.JPG?alt=media&token=b934d062-a5d6-442d-bcff-f0239535f6d3)
9 months agoEx-Google Senior SWE • FE/Mobile -> BE/Distributed/AIIt sounds like you're in disagreement with your manager, which is a good thing! It comes with having diversity. The part that is less than ideal is how it might be affecting your capacity at work, and perhaps even the velocity of your team/organization.
I want to say that pushback in the form of collected disagreement is actually really good even though your supervisor is technically not your peer. It's the type of communication that helps you and your supervisor build a quality relationship despite your differences.
I like Charlie's suggestion - setting up expectations and defining a mutual contract, especially with other teammates involved, will be valuable.
As part of that, I think being able to express the following in a conversation between you and your supervisor will also help:
- Express how and why nits matter to your supervisor. In order to communicate this, you have to understand where your supervisor is coming from. Expressing this will help your supervisor see that you understand their motivations for nits exactly which shows empathy.
- Express the tradeoffs that their view contributes to the overall team/organization.
- Suggest your opinion, perhaps backed with credible articles that you've found.
It may also be worth checking in to see how other teammates feel about this as well. To do this well, approach it as a third party viewing the overall productivity of the team as opposed to a first party that is frustrated with your work environment.
- 09 months agoSoftware Engineer [OP]Early-stage startup
Thanks, these are great points especially about approaching it with a third party perspective.
- 19 months agoTech Lead @ Robinhood, Meta, Course Hero
So nits can feel overwhelming (I have been on both ends), but they're honestly not too bad on their own. The important thing is revisions. If all the nits come in 1 wave, you just make the changes (there's a lot but each one is tiny), submit the revised diff, and it gets accepted - Not too bad.
The problem is if the nits are thrashy and come in separate waves where after you fix the first round of nits, it turns out that your boss forgot a bunch of other nits and drops another pile of them on you. This is bad, and it's bad in general, not just for nits. At this point, you should give gentle feedback to the reviewers that you would really appreciate them reviewing the entire diff thoroughly the first time. You can try meeting them halfway as well: Maybe the problem is that your diffs are too large, which is why they can't catch everything on the first pass.
If you haven't already done so, I highly recommend going through the PR section in my Code Quality course: https://www.jointaro.com/course/level-up-your-code-quality-as-a-software-engineer/one-diff-one-thesis/
