161–170 of 304 posts
As a manager, it drives me crazy when some feature doesn't make it into the sprint because some "senior engineer" decides that this is the time to teach a junior/intermediate the proper way to do something with a long drawn out "teaching process" via PR comments. it's especially galling if the original PR was working, reasonably well written, no major flaws and they forced the junior to rewrite because it wasn't "bes…
Earlier quoted context omitted.
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 correc…
> 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 jun…
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.
Earlier quoted context omitted.
> it's especially gauling if the original PR was working, reasonably well written, no major flaws [...] wasn't "best practices". You see a clear dichotomy between well written code and what your senior engieers see as enforceable best practices. I'd guess you also see half of the time your team spends on coding as some useless nitpicking you're letting them get away with by sheer generousity or bcause you don't have…
> You see a clear dichotomy between well written code and what your senior engieers see as enforceable best practices. No, in general, I agree with code review comments my senior developers make. I'm talking about the difference between "best practices" and actual best practices or in the difference between "this works, but next time, a better architecture would be x, let's refactor next time we come back to this" an…
When I hear comments like this from managers it’s generally a sign that I need to start planning an exit strategy.
edit: Also, “my senior developers?” They’re human beings, not your property, dude
Earlier quoted context omitted.
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 correc…
(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 heuri…
Holy shit. I should have just written this.
Earlier quoted context omitted.
I've always had a pretty positive attitude towards code reviews (still a junior dev), because I know there's probably something I missed or a technique I'm not familiar with or some language quirk I didn't know about. If I submit a PR with a bunch of new functionality, I'm going to be way more concerned if I don't get a handful of comments on it than if I do.
Frankly I think everyone should have this attitude, seniors as well. (I try to) I am experienced but I know I write bugs and code that may be not as clear as it could be. We all do: we're only human. Also depending on the codebase (and language to some degree), if someone senior writes a lot of overcomplex or abstract code that the rest of the team can't understand/maintain, that's just as much a problem as anything…
For me this is a much bigger concern than subtle changes to method names. My biggest headache digging into new codebases is when I run into layers and layers of abstraction that save 3 lines of code but force me to construct an entire mental map of the codebase before I can understand how anything works.
Any tips for giving feedback to senior people here? A lot of the complexity from abstraction is very hard to quantify and experienced people can have arguments that sound reasonable.
As a manager, it drives me crazy when some feature doesn't make it into the sprint because some "senior engineer" decides that this is the time to teach a junior/intermediate the proper way to do something with a long drawn out "teaching process" via PR comments. it's especially galling if the original PR was working, reasonably well written, no major flaws and they forced the junior to rewrite because it wasn't "bes…
You need to rein in these people ASAP, honestly they are the worst kind of coworkers. Toxic positivity and wasting time/effort 99% of the time. We're paid for our skills, and one of those skills is being able to figure things out on our own or ask for help with pertinent questions. This hand holding crap to get eyes on you is just silly and something I really dislike working at big corp.
Earlier quoted context omitted.
> 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 jun…
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 disagree with the title but found myself agreeing with many points in the article. “Don’t be condescending” seems like generally applicable advice. But IMO, sometimes you just know something the code submitter doesn’t (or vise versa) and discussing that can be useful. And i think that’s pretty much teaching!
"Do [X]"
Now I write:
"I see [problem Y], consider making [change X] to improve it"
If the reviewee agrees then the change is easy and straightforward to make, but if they're unconvinced then the phrasing invites a dialog.
Or if I think I see a bug, I'll phrase it like:
"I think there's a bug here, how does this method behave if foo is null and bar is the empty string? I think we'd throw a null pointer error. Recommend adding a test for that case"
Clear, actionable, refutable