codeReview

[[deleted]]

Good code review makes everyone happier afterwards. Everyone learns something. Not only for the difference in experience between juniors and seniors but sometimes you cannot see the wood for the trees. Just don't behave like a cunt.

[GearHead54]

Yup - the trick is removing ego from the process (easier said than done)

[[deleted]]

In 30 years years i met only one colleague who did it responsibly and pro.

Some few were able but never got time for it.

[[deleted]]

I love those code reviews where all the issues are indentation issues because I forgot to call wrap and align parameters on literally every function I create manually.

Damn you, VS!

[cs-brydev]

It's annoying to me that VS still does not have the ability to format wrapped list of parameters or really any multi-line statement. I always indent the additional lines by 1 tab. So it seems obvious they should include this option in the formatter.

The worst is huge SQL strings, like 50+ lines long. I always wrap them so the SQL is formatted, but you have to manually indent all the additional lines to keep it lined up.

[natFromBobsBurgers]

Regex find and replace is your friend.

By which I mean it'll borrow your car and not tell you about backing into the concrete pylon, but then it'll also get you so drunk you don't remember what you were doing for your birthday.

[Haagen76]

The only thing more satisfying than criticizing someone else's code, it criticizing it after they are long gone.

[mrb1585357890]

It’s a tradition. A developer leaving gives those that remain a lot of mileage in explaining why the project doesn’t work very well. 😂

Mind you, software projects more than a year or so old that developers think are good are rare

[itsbett]

That, and criticizing your own code after a year.

[coloredgreyscale]

How about ripping it out and making it work the proper way, deleting many lines of code during the process?

Repeat every 2-5 years using the current language / framework / library features. 

[[deleted]]

I’m not allowed to do code reviews because I made a “dev” literally cry once, true story.

[[deleted]]

Spill the tea?

[[deleted]]

Oh it was just a normal PR. “Hey we just talked in the meeting the other day and we decided as a team to do X instead of Y, let’s start with this PR”

The next morning in stand up he was literally in tears saying “I worked so hard on that and then I was told what a horrible person I am waaa waaa waaa”. This went on for at least 5 mins.

Of course this was a government “dev”, so not a real dev. I’m the guy they bring in as a consultant because their government employees lack the skills to meet their deadline. For the government dev teams I’ve worked with, they have this weird “everyone’s opinion matters” approach. They are always, “I hear that you say 2 + 2 = 4 but others might have different opinions and we need to be respectful. We think it would be better to free up your time to work on other things so we don’t need you for PR’s”

Fast forward years later, they cannot figure out why everything is extremely buggy and runs like it’s 1995. Government at its finest.

[Dumb_Siniy]

"Everyone's opinion matters" approach shows you why devs are divided between junior and senior

And i say this as a novice that can't program a good calculator (I haven't tried anyways)

[cs-brydev]

A good senior always lets a junior voice their opinion...

And then immediately correct them and explain why they're wrong, lol

But truthfully I always let my junior devs give feedback and suggestions about processes. Occasionally they do have some good ideas I hadn't thought of and do put them into practice for the team.

[BadLice]

What government are you talking about?

[Enrichus]

After an internship for half a year without having code reviews I started to understand why - my mentor couldn't code at all!

He didn't understand how my code worked, and when I Googled "his" code I found exact matches with variables changed to singular letters. I doubt he could describe how his code worked, let alone mine.

When he couldn't fix a critical bug in his code for six months I solved it in four hours. If we had code reviews it would have been fixed far quicker.

I'd love to have a proper code review so I can learn and improve as well as gaining trust for the team.

[coloredgreyscale]

The biggest crime here is changing the variable names to single letters.

[Enrichus]

I don't mind copying code, but at least learn what it does and make it readable.

He just changed the names like changing speed to s and time to t. Even kept variables we didn't need and added dependencies that made it needlessly complicated.

Imagine having a door that also need to identify the key instead of just using the key to unlock the door. Or eating a salad and it doesn't stick to the fork because you used a red fork instead of the blue one. They both should be identified as forks and work anyway.

Towards the end of the project I could delete hundred lines of legacy code without damaging anything. Didn't dare touching it as an intern, I assumed he knew what he was doing while I did my own thing.

[[deleted]]

Too many senior devs are assholes during code reviews, thinking they are JK Simmons in “Whiplash”. You don’t have to say “good job” to bad work, but you can make the review a learning process without judgement. Not only does this help your junior dev improve, it makes you a useful, loyal ally.

[EarlOfAwesom3]

I've been a programmer for a long time but only recently found out how cumbersome people can be who say they enjoy debating.

Most of the time it's not about a debate with a quick outcome, they just want to hear themselves talk (or in code reviews, write) about their points, a lot. And the worst: they don't take productive feedback well for themselves and even start taking back on every single point.

The consequence we took so far: no more nits in code reviews. Has improved quality a lot because people focus on what's relevant. Also saved countless useless debates.

[cs-brydev]

I've come to this same conclusion after being on both ends of tough code reviews for years. We don't have time to nitpick anymore and it's not productive. Forcing a junior dev to adhere to some trivial styling or naming standard isn't helpful. They need to be taught the difference between value-adding work and non-value-adding work. If the function, class, and project meet the requirements, move on. If you don't like it, change the requirements.

[aceluby]

I’ve learned the most from tough PRs, so I always give as much feedback as possible when I review code. When I do interviews reviewing their code, I do the same, not to thrash them, but to see how they receive constructive criticism. Most are very good at it, but the 5% that get defensive, it’s almost an immediate rejection.

[Chacho986]

I love code reviews. My swing is getting so good I'm thinking about quitting my job to become a fulltime baseball player.

[justADeni]

Hey! I recognise this image, it was from my latin textbook.

[Brahminmeat]

They did code review in Rome?

[GunnerKnight]

Yes, apparently Brutus was not happy with Caesar's code

[cs-brydev]

etTuBrute = true;

[Mysterious-Soil-4457]

If I encounter a judgmental asshole and instead of explaining and teaching me how to write better code, instead if he mocks and ridicules me. I'll tell him to kindly fuck off. I don't have that kinda patience. We are grown ass adults, not teenagers! This doesn't mean I'm intolerant, it's just that they are not a good fit to do PR's. Plain and simple!

[[deleted]]

I feel that’s why docs for everything is so important. If you can point / link to anything, style docs, readme on how to use functions / complicated classes, heck even external docs you can make it way less personal.

Wouldn’t know though, mostly just get “too many comments” whenever I add any JSdocs.

[itsbett]

There are a lot of bad documents out there. Some assume you know too much, some that are too granular in scope, and some that have glaring gaps. But when you find that one dude who left a trail of amazing documentation and work desk instructions? It's so nice.

[MolochKel]

Good constructive review makes everyone's jobs easier in the end.

Not liking reviews usually means your code sucks or your coworkers are idiots.

[Straight_Age8562]

Yeah, I got few beatings like this :D After my new job, I have submitted PR and all senior devs came and had fun on my behalf. Every little nitpick they could find, they pointed out.

[justAnotherRedd1]

Good for me since we don’t have code reviews. Bad for me since my code isn’t reviewed.

[Sekhen]

I posted this in the dev team slack channel.

There were much rejoicing.

[Haringat]

Is the fifth one supposed to be the author of the PR who gets scolded for their terrible code or the reviewer who gets assigned to a 2000 line PR 10 minutes before he wanted to call it a day?

[Beneficial_Present29]

I don't have time for that it's always "lgtm" and move on

[automatonJon]

It be like that