0

TL;DR; Should I explain to junior developers that I'm being scrupulous during code reviews because our boss asked me to help them improve their code quality, and not just because their code sucks for technical reasons?

(I don't really say "your code sucks" during code reviews, this is just a TL;DR; version. The long version typically goes along the lines of "I think X should by Y because of Z, and also this and that".)

I am a senior developer in a large organization with a somewhat formal employee performance evaluation process. In my current project I play a lead role for a handful of junior developers, organizing sprint planning and software design, conducting regular code reviews etc. It's a small team so typically all are present at every activity.

At the start of the project our manager, in a private conversation, asked me to help ensure those junior developers improve the quality of code they write, things like following style guides, adding useful comments and JavaDoc, creating thorough unit tests and such. I'm assuming this will be taken into account during their performance evaluation later.

As a result I've been trying to be quite rigorous in my code reviews, and I suspect some of my comments might have been seen by some of them as unnecessary nitpicking1.

Sometimes I meet certain resistance to my suggestions and I have to spend quite some time explaining why we need proper JavaDoc and concise method names, and why we should fix the code now and not "later". I can successfully get my points across with technical reasons, and we reach the state that seems acceptable to me, from both points of view: we get the code quality we need to deliver a proper product, and I help our manager to meet the goal of helping the junior colleagues to grow.

I think that if the junior devs knew the deeper reasons for such thorough code reviews we would spend less time arguing about them and more time working towards our target release date. So, the question is, should I explain to them the "political" reasons for my rigour, or stick to the technical reasons? Or would it look like an attempt an an "argument from authority"? If I should, would I do it in front of the team, or approach each person privately?

1 - Some examples:

  • A class constructor JavaDoc has @param <name> but no description, so I insist they put a useful description for each parameter.
  • A comment inside the code is vague and unclear, so I suggest they either remove it if it's unnecessary, or change it to something more useful.
  • A method of an Apparatus class has a single method to shut it down, called shutdownApparatusSafely, so I suggest shutdown should be sufficient (more concise and to the point).
Improve this question
10
  • Does this answer your question? As a mentor, how to interact with the team to adopt my thought processes and suggestions? – gnat Sep 18 '20 at 20:27
  • 4
    For the last example, are there multiple methods to "shutdown" an Apparatus? The "safely" part of the name makes it sound like there is also an "abortApparatus" method. – DaveG Sep 18 '20 at 20:37
  • @user123 Welcome to The Workplace. I suggest you register your account so you can edit and provide clarifications. I also suggest you take the tour and read the help center so you know your way around here. – DarkCygnus♦ Sep 18 '20 at 21:22
  • 4
    If I call a method "shutdownSafely" then you can bet that there is a reason for choosing that name and not "shutdown". – gnasher729 Sep 19 '20 at 8:53
  • 1
    Your suggestions don't sound harsh to me, though I'm a junior; my role is learning from more senior members, and understanding what the company wants, not arguing about it. An empty comment would make me wonder what bits of empty code are lying around. There are always ways of saying things like "building you up", "bringing you to your full potential", "best it can be" that have less of a critical tone but underline the fact that less-than-optimal code is not accepted. – Denis G. Labrecque Sep 19 '20 at 13:29
9

You shouldn't tell them "I'm being harsh because my manager asked me to be harsh". YOu should tell them why these things matter- long term maintainability, readability, the costs associated with changing it later (Mythical Man Month type things). Teach them why this is important. That's all technical, not just politics. If you can get them to understand that, they'll become better devs as well.

(Although shutdownApparatusSafely may not be too bad if there are also unsafe ways of shutting it down, or if historically it wasn't being shut down safely and this would make it more obvious that it needs to be. I'd probably keep this name.)

Improve this answer
6

Should I explain to junior developers that I'm being scrupulous during code reviews because our boss asked me to help them improve their code quality, and not just because their code sucks for technical reasons?

Yes.

I suggest you don't say "your code sucks for technical reasons", that wouldn't be professional nor polite.

But yes, do inform them for the reasons of you being scrupulous on the reviews, so they are aware that your boss asked that and don't have to assume.

Improve this answer
0
1

Should I explain to junior developers that I'm being scrupulous during code reviews because our boss asked me to help them improve their code quality, and not just because their code sucks for technical reasons?

Sort of.

It would make sense to explain that part of the reason for performing a code review is to help developers improve their code.

But saying "The boss wants me to help you improve your code. I assume it will be part of your performance review." wouldn't be appropriate.

If your boss wanted to convey that to the developers, your boss would have told them directly.

Improve this answer
1

Ask your boss.

Maybe he’d prefer you to tell them, maybe he’d prefer to tell them himself, maybe he’d prefer for you to leave them in the dark on the political stuff and focus on the technical stuff. Your boss is the one who has given you this task, and none of us are able to read his mind to tell you how he wants it performed. If you’ve got a question about it, ask him! Clarifying the requirements of the tasks his subordinates undertake is literally a part of his job.

Improve this answer
0

Should I explain to junior developers that I'm being scrupulous during code reviews because our boss asked me to help them improve their code quality

This is why you do reviews, so this explanation is like "in the review our boss wants me to do what a review is meant to be" :-) Mentioning "boss" adds unnecessary pressure.

For someone nervous and feeling in front of court, point out that one often would like to ask someone else for an opinion but one doesn't because it consumes two peoples' time.
In a review not the reviewer forces the author to reveal code and take all the blame, but the author gets a chance to ask for an opinion and talk about things without feeling unproductive or stealing someone else's time.

Explain that an author who wrote and modified something all over has too much overview on the whole, so it's virtually impossible for them to detect where someone else could have problems to understand it.
That's why the reviewer comes in as someone who hasn't seen all that before and has not all the author's implicit thoughts that help getting along with that code.
Finding things to be improved is an expected outcome of a review, not a sign of bad programming.

Improve this answer
-3

To me this seems like you are trying to game the system for them. You should stop doing that.

If you recommend changes you're not convinced improve the code quality, that will shine through and demotivate them and have them think less of you. If you are convinced your suggestions do improve the code quality, simply argue along those lines and explain it to them. You want them to learn not beat the manager's test.

If you argue well and they still make a fuss about you being nitpicky, they deserve having a few malus points on their evaluation, because they clearly lack professional skill or conviction. Wanting to improve your code quality and recognizing what is an improvement is a fundamental part of being a professional software developer. Same as bridge builders should aim to make their bridge last and painters should aim to get the colour perfect without dark spots. That doesn't mean they cannot change, but it's totally fine if that is made clear in an evaluation, that's what evaluations are for! Especially when they are juniors. If you protect them from negative feedback they have even less chance to adapt or recognize that they might be in the wrong job.

You surely can bring the fact that code style is part of the evaluation, but typically that should happen during said evaluation meeting. And it brings in the wrong motivation. Sure, discussions might subside, until they make it through the next evaluation or are sure no one is looking or can attribute the code to them personally. The core motivation should be to provide quality code. If the rules for evaluations haven't for some reason been made clear, make it clear once - perhaps in a general discussion round about what it means to be a good developer.

Therefore, in most cases I'd suggest to avoid the evaluation context, because it creates a school exam setting where you are the evil grade master and they need to get around your grading scheme. Discussing it personal 1-1 also makes this personal and each junior might think you singled them out and feel pressured. So if you do it make sure it comes across as a general topic. This is less an issue if you are having 1-1 meetings anyway and you're managing the juniors in general, because you then should regularly address their performance anyway.

Improve this answer

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .