Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Don’t teach during code reviews

michaelagreiler.com

Post reply on HN
Print view View original

271–280 of 304 posts

Re: Don’t teach during code reviews

#271

Post by andrewflnr »

nonethewiser wrote: post #217
wk_end wrote: post #71

Earlier quoted context omitted.

> Because to become teachers, you typically don’t have to have your ego brutalized jumping through irreverent hoops. No, instead you have your ego brutalized by spending half your youth (not to mention tens or even hundreds of thousands of dollars) getting an undergraduate and master's degree and teaching certification...only to receive poverty wages, pay for your own supplies, be abused by students and parents and a…

What kind of idiot would choose that life for themselves?

Idealists. People who care about making a difference more than about money. You know, nice people.

Re: Don’t teach during code reviews

#272

Post by throwaway892238 »

tremon wrote: post #142
throwaway892238 wrote: post #62

Earlier quoted context omitted.

Ask them if they want a lesson first. Not every junior wants their superiors lecturing them.

Yes, and not every kid wants to be told what to do by their parents. But it still happens, because that is part of the parents' role. As it is with senior developers.

You just described Paternalism...

Re: Don’t teach during code reviews

#273

Post by rendaw »

moomoo11 wrote: post #63

Hm. So I just give my opinion on code reviews by just giving explanation, link to a documentation, along with my recommended change. My job is to ship software, and as an engineer/team lead it is my job to ensure the code we ship ( I think people take code reviews way too personally. Like, my priority is to ensure I don’t have headaches down the line and when I work with peers or more experience people, I barely leav…

Unless the submitter is stuck or asks for it, I avoid giving recommended changes on code reviews and instead just explain what (bad) consequences pieces of code have.

Providing recommendations takes away autonomy from the person who wants to contribute, and it's easy to end up glossing over the real consequences and ending up with "well, I would have done it this way" nit picking reviews.

Re: Don’t teach during code reviews

#274

Post by l0b0 »

> Today’s one is about how (not) to “teach” during code reviews.

> It’s not bad to “teach” in code reviews, but it should happen on an eye-to-eye level.

> But first, let me show you how I’d phrase the feedback for this example:

> “I had a hard time grasping what the method does. What about changing the method name to openRequest() to make the methods objective clearer and improve code readability?”

So the headline is just wrong, it's about not being a dick or vague when writing code reviews.

Re: Don’t teach during code reviews

#275

Post by [deleted] »

moomoo11 wrote: post #63

Hm. So I just give my opinion on code reviews by just giving explanation, link to a documentation, along with my recommended change. My job is to ship software, and as an engineer/team lead it is my job to ensure the code we ship ( I think people take code reviews way too personally. Like, my priority is to ensure I don’t have headaches down the line and when I work with peers or more experience people, I barely leav…

[deleted]

Re: Don’t teach during code reviews

#276

Post by dilyevsky »

Ensorceled wrote: post #130
doutunlined wrote: post #124

Earlier quoted context omitted.

Why would you rewrite them as loops?

"It's much easier to read a loop than a list comprehension, please rewrite all the list comprehensions as loops." I squashed that.

List comprehensions are advised against in Google’s official Python Style Guide and many similar guidelines for this exact reason. You sound like a micromanager who thinks he’s more technical than you actually are

Re: Don’t teach during code reviews

#277

Post by throwawaymaths »

pavlov wrote: post #32
throwawaymaths wrote: post #27

Earlier quoted context omitted.

Gp probably meant on the social dance floor which is generally considered to be taboo, and doing so is a mark of either a jerk or a noob.

It really is surprisingly infuriating. A good friend of my wife once did this to me at a wedding. She had started a pair dancing hobby maybe six months earlier and was very enthusiastic about it. When I asked her to dance, I didn’t mean I wanted a lesson. It was so out of sync with my expectations that I left with barely an excuse in the middle of the song.

I just remembered I did recently ask someone to teach me a dance ("just off" the social floor), but 1) it was in a crowded bar just off the dance floor so it's less in the way than the onlookers 2) it is a notoriously easy dance 3) she knew I was good at dance 4) I picked it up in 45 seconds 5) we immediately went on to the dance floor and finished the song together with no further instruction.

So there are exceptions to the rule

Re: Don’t teach during code reviews

#278

Post by razemio »

fzeroracer wrote: post #249
hrayn3 wrote: post #183

Earlier quoted context omitted.

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…

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 nee…

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.

Re: Don’t teach during code reviews

#279

Post by ornornor »

turdprincess wrote: post #246
bcrosby95 wrote: post #209

Earlier quoted context omitted.

I'm gonna call bullshit on all of this. The time tax on not leaking layers in an application that doesn't already leak layers is nearly zero. The problem is people don't want to expend the very minor extra amount of effort. So you do it once. Then the next person does it because hey we already do it there so the code is not even subjectively worse, it's objectively the same, cuz you already crossed that line. After a…

In my experience, codebases never maintain their level of quality. Unless you are taking deliberate action to improve the codebase with every commit, it's actively getting worse like you described above. The interesting thing is that this isn't a technical problem, it's a human problem. Most folks want to clock in, do their job and clock out, and going the extra mile to improve things isn't worth the effort. I think…

> Most folks want to clock in, do their job and clock out, and going the extra mile to improve things isn't worth the effort.

I suspect this is because it’s almost never rewarded. Either because nobody up the chain of command understands and values quality beyond “it’s working and making us money right now so it’s fine” and also because doing things right is tiring. Why should I spend energy on this and not have that energy when I’m off the clock if I’m not going to get more money/recognition/time off for it and I’ll probably get laid off or move on in a couple years anyway.

I don’t think anyone deliberately starts a job or task thinking they’ll do the absolute minimum. People generally want to do their best but the system moderates this desire and that’s the result.

I don’t really have a solution, just pointing it out.

Re: Don’t teach during code reviews

#280

Post by theonething »

bcrosby95 wrote: post #209
simplotek wrote: post #185

Earlier quoted context omitted.

> They are aware of things like the separation of concerns, but as soon as they run into a situation wheres its easier for them to leak one layer's concern into another they will do so. That says nothing without context. If you want to move fast then it's ok to break things, and leaking layers is far from a non-negotiating tradeoff. Software is soft, and you can always revisit a piece of code to refactor it to suit s…

I'm gonna call bullshit on all of this. The time tax on not leaking layers in an application that doesn't already leak layers is nearly zero. The problem is people don't want to expend the very minor extra amount of effort. So you do it once. Then the next person does it because hey we already do it there so the code is not even subjectively worse, it's objectively the same, cuz you already crossed that line. After a…

In my professional experience of > 10 years, that is 95% of codebases. Maybe I'm unlucky?
Post reply on HN