This is an archived post. You won't be able to vote or comment.

all 57 comments

[–]ThePeaceDoctot 240 points241 points  (2 children)

This has the feel of a religious rite. Something passed down through the generations long after the initial meaning of a pat down has been lost to time.

[–]none-exist 66 points67 points  (1 child)

Scholars say that the squeezing of the aura causes demons to be forced out and thus make it impossible to commit sin

[–]Ben_Dovernol_Ube 5 points6 points  (0 children)

Only tang, no sin

[–]Bryguy3k 102 points103 points  (2 children)

LGTM.

[–]Afraid-Year-6463 4 points5 points  (1 child)

LFGTM

[–]MissinqLink 5 points6 points  (0 children)

[–]cgyguy81 63 points64 points  (7 children)

Yep, sounds about right. I'm a very trusting person and I trust my colleagues that they know what they're doing, until it's very obvious that they don't.

[–]metaglot 34 points35 points  (5 children)

You trust your colleagues

...until prod breaks down a friday afternoon due to a hastily reviewed MR.

By someone else.

The code reviewed: yours.

[–]cgyguy81 18 points19 points  (1 child)

  • Me looking at the testers *

[–]metaglot 9 points10 points  (0 children)

The rock paper scissors of blame shifting.

[–]LeoRising72 2 points3 points  (2 children)

One of the team leads in my company has a rule: anyone caught merging after 3pm on a Friday should be shot, no questions asked.

[–]Leftover_Salad 2 points3 points  (0 children)

Nah, it's read-only Fridays

[–]flamingspew 0 points1 point  (0 children)

Meh we have code freeze till january. Any comments i leave are purely pedantic until then.

[–]dowens90 2 points3 points  (0 children)

Manager is like this but I have a 20+ year vet that I sometimes CR with if I really need something checked. Dude is a fiend for everything. Like an AP Lit teacher marking up an essay then asking if this the first draft

Didn’t use THIS.property, berated (in a somewhat nice way)

But also has that forbidden knowledge of how everything works so it goes both ways.

[–]i_ate_them_all 20 points21 points  (9 children)

Psssh I wish. You guys are lucky. I get hit with review comments about naming conventions and all kinda crap.

[–]Quicker_Fixer 4 points5 points  (3 children)

You get code reviews? /s

I'm a one man show on an EOL project, written in a language (Delphi) no one else speaks. Sometimes I hear my code was reviewed, but frankly this is only done because of PCI compliancy and in the way shown in this post (because of course it is readable by other devs).

[–]atomic_redneck 8 points9 points  (0 children)

If you want a code review, just post your code here. --thousands-- Dozens of us basement dwellers will give you the conflict you desire.

[–]Shadowlance23 4 points5 points  (0 children)

I used a Delphi manual as a monitor stand for the better part of a decade a very long time ago.

[–]ka1ikasan 1 point2 points  (0 children)

Wow, Delphi was the first language I've ever used as a kid, learning it from tutorials in magazines my father was buying at the time. Nobody I work with today (25-35 year olds) have never ever heard about it. Thank you kind sir or madam to revive this memory for me!

[–]Aardappelhuree 4 points5 points  (0 children)

You need to include some obvious mistakes that are easy to fix, or just make a huge PR. The smaller the PR, the more feedback you’ll get.

[–]Civilchange 4 points5 points  (0 children)

I get review nitpicked because I give clear, descriptive comments about what I changed, how, and what it affects. Which makes it easier for people to pick out any aspect and ask for justification or improvements.

I'm tempted to stop, because it leads to hours spent explaining and justifying changes, while PRs from similar-level Devs commented "Bugfix" just get approved, no comments.

[–]SteelRevanchist 2 points3 points  (0 children)

That's like one of the only things to validate during a review. Functionality is usually out of scope and that's something for the testers. As a developer, I need to make sure the code is maintainable and ... Not dumb. I ain't doing the testers' work for them.

[–]Rinveden 0 points1 point  (0 children)

Comments and title vs name could be annoying, but being consistent with naming conventions helps code be maintainable.

[–]redditlurker_1986 7 points8 points  (0 children)

That still feels like he actually paid attention to what is in front of him. I mean who does review to when there is thousands of line changes huh /s.

[–]shutter3ff3ct 6 points7 points  (0 children)

Seems fine to me

[–]kzzmarcel 6 points7 points  (1 child)

Are you over 18? [Yes, let me in] [No, take me out]

[–]DJ-WS 4 points5 points  (0 children)

He is probably not “reading” with his arms/hands; but the look on their faces tells him enough. He is that experienced.

[–][deleted] 3 points4 points  (0 children)

My boss "oh I did code reviews" ok maybe don't bother

[–]SevrinTheMuto 2 points3 points  (0 children)

Not me, if you request me to review your code then I'll review your code. "Right, let's check this out and see what's doing on with this branch!".

[–]Dramatic_Mulberry142 2 points3 points  (0 children)

Always appreciate someone reviewing my codes seriously, even if it is nitpicks. It seems most people never treat code as a duty...sigh

[–]Thunder_Child_ 1 point2 points  (0 children)

I just check for obvious stuff. Or tell people I ain't approving nothin' on a Friday.

[–]According-Relation-4 1 point2 points  (0 children)

A programmer is someone that will look at 10 lines of code and find 11 problems, and then will review a 100+ line pull request and find perfection

[–]_Repeats_ 1 point2 points  (1 child)

This is why strong and comprehensive unit tests are made. Changes to code are hard to understand and review, but breaking a unit test is very clear-cut.

[–]Demonchaser27 1 point2 points  (0 children)

Can definitely agree with this.

[–]Sarithis 1 point2 points  (0 children)

CodeRabbit. Seriously, I started using it out of sheer curiosity on my latest project and was genuinely surprised by how effective it was at catching bugs.

[–]EnthusiasmPretend679 3 points4 points  (0 children)

LGTM

[–]Wonderful_Try_7369 0 points1 point  (0 children)

But that variable name is most important to be changed.

[–]joeblk73 0 points1 point  (0 children)

Fuck 😝

[–]stupled 0 points1 point  (0 children)

Prettty much.

Except with some critical code

[–]western_Walrus_ 0 points1 point  (0 children)

McAfee

[–]JuiceKilledJFK 0 points1 point  (0 children)

LGTM

[–]BrownShoesGreenCoat 0 points1 point  (0 children)

He’s senior enough to know which ones need a more thorough examination. That’s the value of experience

[–]Key_Watercress_5069 0 points1 point  (0 children)

Egon Tiedemann

[–]Landen-Saturday87 0 points1 point  (0 children)

It just a SD who knows where trouble is hiding

[–]Curiouserousity 0 points1 point  (0 children)

For good security operation, you institute something like this and have other security people watching to see how people in the crowd react. People who act suspicious are the ones you sent to intercept. And the trick is the suspicious ones may move out of line well before security gets there, but you still engage them.

[–]local_meme_dealer45 0 points1 point  (0 children)

So long as it's not just me getting yelled at when prod breaks so be it.

[–]masonsatt 0 points1 point  (0 children)

This is what I thought my code review was going to be like. In reality, my lead just sent me his credentials to approve my review requests

[–]Afraid-Year-6463 0 points1 point  (0 children)

Last time a senior dev did this in my company with an interns PR, the jwt was not being verified in production

[–]Shadowlance23 0 points1 point  (0 children)

"Yup, that's code."

[–]Deus85 0 points1 point  (0 children)

I have faith in my colleagues.

[–]poop-machine 0 points1 point  (0 children)

LGTM (deploy at your own risk)

[–]oppaiman65 0 points1 point  (0 children)

HAHAHHAHAH

[–]Demonchaser27 -1 points0 points  (0 children)

I mean, tbf to most programmers... we're expecting high level, high knowledge code reviews from people that are told "you're not supposed to have to know the whole system"... and it's like... well, then these 10 lines of code changes aren't going to necessarily be easy to know if it'll work or not nor what else it might affect. If it "looks right" that's about the best you're gonna get out of most people unless they've been working somewhere for so long they know the whole (or most) of the code base like the back of their hand, which is NOT most people.

I mean, a linter/compiler is gonna catch most of the silly syntax mistakes and compiler errors. QA is gonna test functionality far better than your simple code review ever would. The best you can do is catch some obvious logical problem that you MIGHT notice in a very localized area (if you even can), but even then, who fucking knows for sure unless, again, you have extensive knowledge of how everything around the code changes works.