Hacker News Reader

Comment by razemio | Hacker News Reader

At least I think so because of your previous comment. Since I can only take this as context their might be a misunderstanding. Still I would like to point out a few thinks, I did not like about your comment.

In my opinion, you are looking at the problem from the wrong angle. If code gets worse (non working) after a code review, the reviewer made some serious mistakes and/or was not guiding his colleague to the correct solution like it should be. Also if working code gets rejected because of bad practice, you should not be mad about it but be thankful that someone caught that before release. If your only measurement of code quality, is that something is working, I would consider that a dangerous practice.

View on HN

(I'm Not OP) - At the end of the day we're all trying to strike a balance between shipping fast and keeping technical debt down. Asking juniors to only ship code that's good as what a senior would write is very rarely the right balance point. Senior engineers must learn to understand when code is "good enough". If you're a senior and you haven't developed this skill you're bringing your team down.

Concretely, a heuristic I rely on is to understand whether the sub-par code being added "infects" the code base e.g. it changes an important interface, it adds a poorly designed interface other code will rely on etc. These are the places its important to put your foot down. Conversely, if the internals of a one-off function or class could be a little cleaner... eh, ship it.

> Senior engineers must learn to understand when code is "good enough". If you're a senior and you haven't developed this skill you're bringing your team down.

Holy shit. I should have just written this.

> If your only measurement of code quality, is that something is working, I would consider that a dangerous practice.

I'm really confused as to why there are so many people assuming I don't give a shit about quality when I have CODE REVIEWS.

Why are you assuming I want them all to just be rubber stamps?

And what God Complex do you have to have that you assume the "senior developer" is ALWAYS in the right, and the junior and the "non-technical" manager just don't respect "quality" at all?

Not OP, but your final comment confuses me -

Given a senior developer, a junior developer, and a non-technical manager, in the context of a code review, the vast majority of the time you should absolutely listen to the senior developer.

If that's not true in your organization, then hiring, leveling, and leadership should all be questioned.

I'm a very hands on, technical manager. But thanks for assuming I'm non-technical.

Assumed? I responded to your comment:

> you assume the "senior developer" is ALWAYS in the right, and the junior and the "non-technical" manager...

> I'm really confused as to why there are so many people assuming I don't give a shit about quality when I have CODE REVIEWS.

Having code reviews is an incredibly low bar. The very fact that you offer this as an evidence might be the reason people make those assumptions.

Is it? Their comment might come from having worked at companies where code reviews aren't even a thing, let alone source control. It's crazy out there, especially if you're nowhere near Silicon Valley.

Your comments are very reminiscent of non-technical managers I've known - they often have a very shortsighted view of the value of code reviews, because they don't need to work on the code.

'It works - why don't we just merge it? Keep velocity high!'

A code review is exactly where it's worth spending time making sure: the code is maintainable, doesn't degrade the quality of the repo, and above all teaches the junior things they can use next time to do a better job faster.

Spending some time using a review as a teaching experience pays so many dividends later. People who don't touch the code don't understand that.

Of course, there's a level of 'good enough' a senior should be able to identify and approve. But the bar should be high.

Your and some of the other comments I've read are very reminiscent of some of the worst senior developers I've worked with, because they treat code reviews as a way of molding the code base to their own personal expectations rather than improving it especially when a junior engineer finds fault in their code. They waste time using code reviews as a teaching exercise that gives bad habits to junior developers that need to be broken later.

Why? His comment was the perfect description of what a senior developer should do during a code review. Code reviews between a senior and junior are teaching exercises and always should be. It is critical and a fundamental step for juniors to be become more experienced. As already written by the comment you replied to, the right balance has to be found, but this is part of the job description if you call yourself a senior developer.

> I'm really confused as to why there are so many people assuming I don't give a shit about quality when I have CODE REVIEWS.

Because code reviews are a minimum. This is like saying "Why are there so many people assuming I don't give a shit about driving safety, I put on my seatbelt!" Yeah, it's because you're driving rashly and putting others at risk. Just putting on a seatbelt isn't the be-all and end-all of the process.

If you want to know, the reason that people are assuming you don't give a shit about quality are your sprint-centric attitude, your putting the word 'senior engineer' in double quotes, and the talk about "well if the PR was working then it should ship".

This gives off an impression of someone who doesn't understand at a fundamental level that the process of code reviews and have the contribute to learning on the team is is what makes for quality in the long run, not just the mere fact of having them.

Of course, it's quite possible you are the reasonable party here and your senior engineers are curmudgeons. In that case, have you tried talking to them about it? What did they say? Your complaint here seems to indicate that this hasn't happened yet.

Reply on news.ycombinator.com