Join devRant
Do all the things like
++ or -- rants, post your own rants, comment on others' rants and build your customized dev avatar
Sign Up
Pipeless API
From the creators of devRant, Pipeless lets you power real-time personalized recommendations and activity feeds using a simple API
Learn More
Search - "code-review"
-
If Gordon Ramsay made code reviews, I would watch that show. Especially the insults he would use for handling clients.
"This code has so much spaghetti, it decided to open it's own restaurant"23 -
This code review gave me eye cancer.
So, first of all, let me apologize to anyone impacted by eye cancer, if that really is a thing... because that sounds absolutely horrible. But, believe me, this code was absolutely horrible, too.
I was asked to code review another team's script. I don't like reviewing code from other teams, as I'm pretty "intense" and a nit-picker -- my own team knows and expects this, but I tend to really piss off other people who don't expect my level of input on "what I really think" about their code...
So, I get this script to review. It's over 200 lines of bash (so right away, it's fair game for a boilerplate "this should be re-written in python" or similar reply)... but I dive in to see what they sent.
My eyes.
My eyes.
MY EYES.
So, I certainly cannot violate IP rules and post any of the actual code here (be thankful - be very thankful), but let me just say, I think it may be the worst code I've ever seen. And I've been coding and code-reviewing for upwards of 30 years now. And I've seen a LOT of bad code...
I imagine the author of this script was a rebellious teenager who found the google shell scripting style guide and screamed "YOU'RE NOT MY REAL DAD!" at it and then set out to flagrantly violate every single rule and suggestion in the most dramatic ways possible.
Then they found every other style guide they could, and violated all THOSE rules, too. Just because they were there.
Within the same script... within the SAME CODE BLOCK... 2-space indentation... 4-space indentation... 8-space indentation... TAB indentation... and (just to be complete) NO indentation (entire blocks of code within another function of conditional block, all left-justified, no indentation at all).
lowercase variable/function names, UPPERCASE names, underscore_separated_names, CamelCase names, and every permutation of those as well.
Comments? Not a single one to be found, aside from a 4-line stanza at the top, containing a brief description of that the script did and (to their shame), the name of the author. There were, however, ENTIRE BLOCKS of code commented out.
[ In the examples below, I've replaced indentation spacing with '-', as I couldn't get devrant to format the indentation in a way to suitably share my pain otherwise... ]
Within just a few lines of one another, functions defined as...
function somefunction {
----stuff
}
Another_Function() {
------------stuff
}
There were conditionals blocks in various forms, indentation be damned...
if [ ... ]; then
--stuff
fi
if [ ... ]
--then
----some_stuff
fi
if [ ... ]
then
----something
something_else
--another_thing
fi
And brilliantly un-reachable code blocks, like:
if [ -z "$SOME_VAR" ]; then
--SOME_VAR="blah"
fi
if [ -z "$SOME_VAR" ]
----then
----SOME_VAR="foo"
fi
if [ -z "$SOME_VAR" ]
--then
--echo "SOME_VAR must be set"
fi
Do you remember the classic "demo" programs people used to distribute (like back in the 90s) -- where the program had no real purpose other than to demonstrate various graphics, just for the sake of demonstrating graphics techniques? Or some of those really bad photo slideshows, were the person making the slideshow used EVERY transition possible (slide, wipe, cross-fade, shapes, spins, on and on)? All just for the sake of "showing off" what they could do with the software? I honestly felt like I was looking at some kind of perverse shell-script demo, where the author was trying to use every possible style or obscure syntax possible, just to do it.
But this was PRODUCTION CODE.
There was absolutely no consistency, even within 1-2 adjacent lines. There is no way to maintain this. It's nearly impossible even understand what it's trying to do. It was just pure insanity. Lines and lines of insanity.
I picture the author of this code as some sort of hybrid hipster-artist-goth-mental-patient, chain-smoking clove cigarettes in their office, flinging their own poo at their monitor, frothing at the mouth and screaming "I CODE MY TRUTH! THIS CODE IS MY ART! IT WILL NOT CONFORM TO YOUR WORLDLY STANDARDS!"
I gave up after the first 100 lines.
Gave up.
I washed my eyes out with bleach.
Then I contacted my HR hotline to see if our medical insurance covers eye cancer.32 -
The story of how humans evolved to an asshole.
Code-reviewer: please change 'if a==b' to 'if b==a' as it is easier to read so I can approve.
Code-owner: -_-16 -
My code passed the review today. It is now being pushed to production. I can't express my happiness 😅17
-
@Root has a code review.
CR comment: “Why would you do it this way? It’s awful. Clean it up!”
Totally fair. I had copied the legendary dev’s code, and it was ick. Cleaning it was easy and enjoyable. I cleaned the source, too.
CR comment: “Why would you touch this? It’s outside the scope of the ticket. You could get it working without changing all this.”
Revert…
CR comment: “The interfaces don’t match. Now it’s confusing, and that makes it harder to maintain.”
🤦🏻♀️16 -
I sometimes wonder my English is that bad ...
I reviewed a pull request. I commented on this line of code
a = getA() * -1;
saying that
"I think this logic should be in setA method instead."
When the guy asked me for the review again, the change was
- a = getA() * -1;
+ a = setA() * -1;
...2 -
I've never had a code review.
Eventhough I proposed to my boss that we at least review our intern's code once a week, he doesn't think it is needed.
Our intern writes ugly, shitty code...
it usually takes hours for me to fix his abominations... but yeah, what the boss says is always the best.3 -
Not sure what triggers me more:
a) the fact that nowadays no one can type/spell without auto-correct
b) that this passed code review
c) that no one corrected it in 12 months since it's been committed23 -
Code review, here the simplified version. What the fuck has to be wrong with someone who seriously codes the first variant in production code?!19
-
I just love going through the code review gauntlet doing maintenance work, don’t you?
(“boyscout rule” = “leave it better than you found it”)7 -
Created a function called upDawg and started sprinkling it all over the code base. Waiting to see who catches it in our next code review.
All it does is console logs 'Not much man, you?' -
A week in code:
Mon: Write all the code💻
Tue: Review all the code🔍
Wen: Fix all the broken code🔴
Thurs: Deploy🚢
Fri: Review all the code🔬4 -
{Context: English is not my native language}
The first time my code get a review ( by my boss that time)
Boss: Your code is full of butts 🍑
Me: Eh, What?
Boss: *showing his screen* see that? variables names: validateButt, contactButt, seeMoreButt..
Me: *interrupting him* oh, I mean button.
Boss: I know, just being sarcastic, but it'll be better to get another suffix.
Me: 😐11 -
Code Review
My boss: “where’d you get this code”
Me: “You i copy and pasted from one of your projects like you told me to”
My boss: “oh”1 -
My morning is starting off with a 40,000 line code review of a php project....yeah how's your morning going??!??!!??8
-
The nightmare continues.
Currently dealing with a code review from a “principal” dev (one step above senior), who is unironically called a “legendary dev” by some coworkers. It’s painfully obvious he didn’t read the code, and just started complaining and nitpicking.
It’s full of requests to do things that make absolutely no sense, and would make the code an unmaintainable mess.
• Ex: moving the logic and data collection from the module’s many callers into the module instead of just passing in the data.
• Ex: hiding api endpoint declarations by placing them in the module itself, and using magic instance variables to pass data to it. Basically: using global functions and variables instead of explicit declarations and calls.
• Ex: moving the logic to determine which api endpoint to use, for all callers, into the view.
More comments about methods being “too complex” (barely holds water) right next to comments saying “why are these separate? merge them together!”
Incredulously asking how many times I’m checking permissions and how ridiculous it all is. (The answer? Twice.)
Conflating my “permissions” param and method names with a supposedly forthcoming permissions system overhaul, and saying I shouldn’t use permissions because my code will all have to get rewritten. Even if that were true, and it’s likely not, the ticket still needs to use the current permissions. I can’t just ignore them because they might be rewritten someday.
Requests to revert some code cleanup because the reviewer thought the previous heavily-nested and uncommented versions (with code duplication) were easier to read. Unsurprisingly, he wrote them.
On the same ticket, my boss wants me to remove all styling and clientside validation, debouncing, and error messages from a form. Says “success” and “connection failed” messages are good enough. The form in question sends SMS and email using arbitrary user input for addresses. He also says it shouldn’t be denounced on the server, and doesn’t want me to bother checking permissions. Hello, spam!
Related: the legendary dev reviewer says he can’t think of a reason why we would want to disable the feature for consumers, so I should remove the consumer feature flag.
You can’t make this stuff up.7 -
(Forgot to post this a few days ago. Was just too tired.)
Finally finished the code review from hell.
The patch on top of the PR is +1448 -1114, and nearly all of it is rearchitecting, not moving.
I think I spent six days on it, 4-5 productive hours a day? Seems like a lot. This codebase is a bitch to work in.
I’m spent.1 -
A badass pull request review comment: 'A wise man can learn more from a foolish question than a fool can learn from a wise answer.'2
-
We have a developer that is known for rejecting PR during code reviews.
He sent me a message and asked me to come to his desk to discuss my PR.
He mentioned that he didn't like my solution and suggested to rewrite the code together.
So far so good, he is a senior developer and I'm sure I'll pick something from the pair programming session. He went with his approach and faced some issues that led us to my solution after nearly 2 hours.
I'm not angry because this scenario happened at least 3 times but how do you guys deal with senior developers that are stubborn?7 -
Le me: "my code is awesome! The way I did XY and Z is insanely cool, efficient, and maintainable."
Le Boss: "yeah so let's schedule a code review next week."
Le me : "... fuck, Fuck, FUUUUCK!"
Internal Screaming3 -
When the client writes a 100 page novel about his problems, but never sends the ACTUAL CODE to review.1
-
I was asked by a client to code review their platform built in PHP. The platform was becoming slow and new features built by their current IT supplier broke existing features.
I gained access to the source code... One PHP file (index.php) containing about 80K lines of code... I am impressed and disgusted at the same time. 😂3 -
001 REM Code review
010 PRINT "Nitpick nitpick nitpick nitpick nitpick"
011 GOSUB REFACTOR
020 PRINT "This function is too complicated, break it up"
021 GOSUB REFACTOR
030 PRINT "Why do you have three methods for this? Put all the logic in one method."
031 GOSUB REFACTOR
040 GOTO 020
041 REM ARGH
998 PRINT "Looks good."
999 STOP8 -
Team member just requested a code review in French (s'il vous plaît)
Yea, I'm not reviewing your code you pretentious prick5 -
Actually happened on a code review:
Tech lead: "Why did you remove this code?"
Dev: "Why did you wrote this code?"1 -
I've been looking at the shittiest code today. Hundreds of lines saying
this.thing.otherThing.EvenAnotherThing[this.someFuckingIndexThatShouldntBeAField].theOnlyBitThatsDifferentPerLine.AlsoNoneOfTheNamesWereThisMeaningful
Over and over. They're all wider than the editor window. Clearly copy pasted. Just make a fucking variable Jesus Christ how do you expect anyone to read that2 -
When you and your colleagues fight over code intending whether it should be a tab or ''x'' spaces.
My reply: It doesn't matter what you use!! It's like eating with either a fork or spoon, as long as you eat, dammit!!
My friend:5 -
No Microsoft Word, I'm pretty fucking sure it's spelled Git and not Gilt.
#Preparing Code Review Docs5 -
Friend - could you comment your code, so I can review it pls.
Me - *comments "gets shit done" ,
"Does some shit ",
"I really don't like commenting my code "4 -
All I've been doing at work last few days is code review. Damn, I feel bored. Just give me something to code already!3
-
When a senior developer changes your impeccable code and pushes their ugly indented lines with bad naming scheme without review.
-
So, my boss is pretty cool. Two of my colleagues made a review of my code (me being new, also on job training). We three were sitting in front of my code, me explaining enthusiastically my code, one of my colleagues looked a bit confused. My boss listening to the whole conversation, he said: "Her code works perfectly". But the way he said it, priceless! I swear, he had a very 'bitchy' voice and also waved while saying that. He looked proud, and we started to laugh.4
-
That moment when you replace
If (blablabla) {
return Yes;
}
else {
return No;
}
with
return blablabla;
And it not passed code review because "We should have readable code"2 -
Found this in a code review today.
Technically, I guess, that is one way to fix a divide by zero error.14 -
Going for my first code review. My colleagues and I will read through my main class, which is 832 lines long and a two-three comments.3
-
Boss: we can't accept your MR request until you fix the problems we highlighted, everything is blocked and the client is getting angry
My brother in Christ, I understand your concerns but I need you to understand: you decided to block a perfectly working and documented PR because you didn't like having "<!-- -->" in a couple of HTML files and menial bullshit like that.
It may not be the most elegant thing ever but don't put on me the responsibility of your blocks or I'll smash your face with the coffee mugs I've used to work until midnight so that you could deliver the product in time after someone else delayed the deadline twice already.
Thanks and get fucked ASAP.3 -
First code review ever, and it's for my job.
Guy was really nice and polite.
Even correctly guessed I don't have much experience with professional coding outside my associates degree and prior job where I was the only programmer most of the time I was there.
Said that since it works functionally and is such a small program there's nothing wrong with it if it meets our purposes ( low priority project )
Then he politely in his words 'nitpicks' 3 points and gives me ideas on how to make it more reliable and less likely to need replaced or completely refactoring in the future.
I think my first time getting code reviewed went well. And one of the things he mentioned was something I didn't know how to do and only took 20 some minutes to implement so I also learned something new from this7 -
I don't think I'm smart, but why most of the people are so retarded??
They want to just make the code work and skip home!
Not caring about how their code effects the whole system!3 -
I just did a code review on an old project of mine. 10k lines altogether. 5.7k TODO's. I can't even.4
-
Just taken over a project from a "high end" London agency to find their code is just poorly copy/pasted examples from the WordPress codex.2
-
Sometimes I think that going to jail for arson wouldn't be so bad. But other times I'm not doing code review on a Monday.
-
I once spotted this in our PHP code:
if (!empty("literal string")) { ... }
How did that even passed the code review process still remains a mystery.1 -
100% of the code defects from a recent code review are from code copied from elsewhere in the codebase.1
-
Manager : what is "looks good" in code review comment??? You have to be more detailed.
Me in next code review : It is not aesthetically pleasing, but it gets the job done. -
Sent my changes before everybody for code review, got git blocked because today was demo day, and ... And asshole guy merged his own PR without code review. That conflicted with my PR. I am going to start posting the shennanigans of asshole guy from now on, just to have a record of his stupidity.10
-
Dev: "I've pushed some code. Give it a code review."
Me: "ok, i'll do it"
<<fast forward>>
Me: "Sounds good to me. Only thing, I wouldn't have gone for all those renames because that was not part of the request, maybe we can discuss ...."
Dev: "I like those names and besides, it's already deployed in production"
Me: " :| .... what's the purpose of a code review when you push straight into production ?4 -
Sometimes I like to look at my PR and think "Damn, that's some beautiful code, I can't wait for someone to review it".
Then the review comes and see a bunch of requested changes. =/5 -
you're doing a code review and you ask for a simple fix and the reply you get back is: "that's not my code. I just copied and pasted it from somewhere else."1
-
I had code waiting in review for ten days, blocking other work. On the eleventh day, the final reviewer (who was standing behind me as I wrote it) says "I'm not sure that I agree with the design, here."
I get you, man, I can re-write the algorithm, but I am so not in that context anymore and you've just delayed release of the feature by at least a week. Ugggh.5 -
I AM SO HAPPY. GOT NEWS THAT A POPULAR YOUTUBER JUST USED A REVIEW CODE FOR OUR VIDEO GAME!! I AM DIEING.4
-
One of my TL said to me during code review that place a break statement after return statement in switch case.
Being with a bar leader can certainly degrade your code quality.10 -
When you can't sleep, because you are more worried about your code review...
That's my state right now... -
I was told Friday I had trust issues because I wanted to code review pull requests from outside the team.6
-
Code review, intern style:
Intern: Here is my pull request ...
Colleague: I see a problem with x, y, z. Could cause memory leaks.
Intern: Oh yeah you are correct, i'll fix that in the next one.
Intern: *merged* -
Look at other peoples code, analize it, absorb patterns, let those patterns replace the shit I have to learn in school, review code, code with those patterns, feel weird, because something is missing, repeat3
-
When the PO asks for a last second code change right before the sprint review, and now it's your turn to demo1
-
Usually my workplace is pretty chill, but today something rantworthy happened!
During code review, I found this guy had styled each element inside his components using nth-child selectors. For instance, in a card the heading was styled by nth-child(1), the text was styled by nth-child(2) and so on... No use of actual fucking classnames.
When I pointed this out, he told me it was actually the better way of doing things because classnames increase the size of the HTML document!
He also claimed proudly that nth-child() is more efficient in performance (idk - anybody can confirm this?)
I'm the only "css guy" there so nobody could second my views. Posting it here so that I can show this to him tomorrow by demonstrating what opinion other css devs have on this and prove my concerns / convince him to change his code.7 -
Dear team leader, If you tell me „I need to review this merge request before merging”, then make sure you are able to allocate time to do this.
If you need MORE THAN A WEEK to even start, then maybe your delegation skills are nonexistent. -
If you are in a team, take advantage of their expertise. Code review with them can also save some headaches later down the road.
-
During college, I was unable to compile a program during lab sessions. So I decided to ask the assistance of Technical assistant. He went through my code for like 5 minutes and said :
"You forgot to add Comments"
To which I replied :
"But sir what that does have to do anything with the program"
His reply was :
"DON'T ARGUE WITH ME, DO YOU WANT ME TO DEDUCT YOUR LAB MARKS"4 -
Just committed a code review change with a heart emoji included, Turns out Crucible does not support this and it broke the code review, Spent the last half an hour trying to change my commit message to fix the review
FML6 -
Code review?......... what's that?
Joke apart, We literally don't do that in my workplace, only few people care about code quality and what they are pushing to production2 -
Last 4 days, struggling to get ship it from a Dev who is reviewing my code.
The comments have already piled up more than the LOC submitted.
The code review consists of just 2 interfaces and a pojo. Hardly 20 LOC in total, excluding javadocs.
I hope it gets ship it soon.
Wish me luck.2 -
So there is a 50/50 chance I am getting flamed af tomorrow during code review because of my branching/merging actions on thursday and friday... Merge conflicts... We all love them...3
-
Team outing.
Planned to send out a code review before the trip. Everything was ready.
Then, someone pushed new changes, and I got damn stuck in fixing the UTs.
Currently, ranting from the resort, missing the commit that wouldn't reach a code review.
I expected to finish this task in 1 hour. Well, I should never ever estimate things.
Note: Played a grand piano though.. yeahhhhh.. -
Do not, I repeat... DO NOT approve a pull request that you cannot adequately review. You're more to blame than the person who wrote the code in the first place when it fails.
-
QMS admin: you only finished the code review, you didn't complete it!
Me: opens review clicks complete
QMS: you didn't export the code review comments!
Me: opens code review again. Clicks Export. Attaches *.txt
QMS: you exported the comments in the wrong format, I can't read them
Me: what is the right format?
QMS: SOP document <random alphanumeric> clearly states the format
Me: spends 20 minutes navigating the piece of crap QMS software with no search function folder by folder.
Finds document.
It's 120 pages and 4 years old.
On page 68, I find "template to be implemented"
Reply to QMS, that document doesn't actually give a reference to a template
QMS: Email my line manager "Please teach your staff how to do a code review"3 -
Random code review:
contractor changes 2 lines in the .gitignore and 1 line in the composer.json and logs 4.5h against the related ticket .. hmmm2 -
Testers in my team have been told like 1000 times to follow the style guides that we all follow. That's not that big a deal. The big deal is that they were put on this project without having any mathematics background when the project is all about geometric stuff. So after me as a developer having to put so many hours to explain to them why the tests are not covering the requirements or why the tests are red because they are initializing the data completely wrong, I ask them pretty please to do the checks for the coding style and I have already been 4 hours reviewing code because not only I have to go through the maths and really obscure testing code to ensure that the tests are correct, but every line I have to write at least 4 or 5 style corrections. And some are not even about the code being clean, but about using wrong namespaces or not sticking to the internal data types. For fuck shake, this is embedded software and has to obey to certain security standards...3
-
Dev sent out a code review request.
I take about an hour, ask questions, make suggestions, general feedback, etc.
Today I noticed none of my questions were answered, developer closed the review, and the code merged into the production branch.
So I email him, asking him why the review was closed and why none of my concerns were addressed before merging to production.
Dev: "No one responded or left feedback, so I thought it was OK to merge up."
Me: "I reviewed and left feedback within the hour you sent the request."
Dev: "Oh yea...you did. Sorry. The code is already in production, but if you still want to leave feedback, create a work item, and I'll take a look."
No you won't.
An example of the code...The dev added an async method to a test harness *console app*. Why? .. check in comment was "Improves performance and enhances the developer experience.."
NO IT DOESN'T!
OK..that's off my chest. No one is getting punched in the face today.6 -
Perform code review and see stuff like this...
var count = dbContext.Posts.ToList().Count();
Selecting millions of rows from database just to get a count...4 -
The best code review experience I had is when I started with my first job I used to write 10 lines of code and used to get 20 comments on that, well i learnt a lot from him and now whenever i get review comments on my PR i actually feel good.1
-
If you have to review the code of other devs and they make the same mistake every f*cking time. But you are a friendly colleague so you try to keep calm and and remember them politely.
And so the only place to rant (and use the word "f*ck") is an app :(1 -
Last year in my job, I was temporarily assigned to another team to help out in their project as they were short-staffed. It was a massive project and of course there was a lot of code review to be done. But since I was only temporarily assigned, I still have to do code reviews for my base team, this other team I was assigned to, and for some reason, code review for another team that I barely know what their project is about.
There were times where all I was doing was code reviews that took anywhere between a few minutes to upto 3 days. The amount of mistakes and bugs I kept finding was phenomenal. But I think the one thing that got to me was finding the same bugs/mistakes that I kept pointing out to people to stop doing or to fix e.g DB queries inside a loop just to retrieve data.
To this end I still have to deal with the same issue, but thankfully now it's only to one team.1 -
I had a ticket to enhance the loading of a page.
So instead of doing 40K requests to a MySQL DB in order to generate a tree and display it to to the user on each page visit, the initial query was optimized and moreover, the results are saved in a MongoDB which will then are served to the user on each page visit.
Long story short, after a code review the code got shipped to production and there was a bug which got fixed in a Hotfix shortly afterwards.
I got all the blame for the bug.
I don't deny I have a responsibility for the bug.
Do you guys think the code reviewer also has a shared responsibility for the bug?4 -
So I've been just assigned a code review. For a complete new feature.
It's over 20k lines of Code.
Time to quit -
Work rant :
I once had a code review and remembered I forgot to comment my code and said sorry I forgot to comment it out.
The reply I got?
Don't worry, here we say your code should be readable enough and no comments are required.
Im still amazed, like... Even if the code is readable, fuck this I need a tl;Dr comment for the long ass fucking code... What the fuck5 -
Do you all look for code complexity O(n) while coding? Or you make sure that your code runs and never look back what's happening ?
Because as per code review no one looks for code complexity and that's so sad11 -
Typical Git work flow on a feature branch:
Commit#1 : The silly feature itself that took 10 minutes to code
Commit#2 : Added unsaved files
Commit#3 : Fix unit tests
Commit#4 : Fix
Commit#5 : Fix
Commit#6 : Fix
Commit#7 : Various Fix
Commit#8 : Added unsaved files
Commit#9 : Merge
Commit#10 : Fixed unit tests
Commit#11 : Code Review tasks
Commit#12 : Revert- Code Review tasks
Commit#13: Refactor part 1
Commit#14: Refactor part 2
Commit#15: Deleted unit tests
Commit#16: Added checking for null
Commit#17: Completely different feature's bugfix
Commit#18: Code review spacing corrections
*Approved*
Trying to merge, then merge conflicts.....2 -
to whomever it may concern...
if i wanted to do code review keeping in mind how asshole you have been and made it my personal vendetta, i would not review it at all.
i would let you and your shitty code rot in hell. -
"What in the name of hell? Why? No.. absolutely not. Jeeeesus. Holy cow!! Haha.. that's funny. No friggin way! Oh that makes sense..wait, that makes no sense. Screw it I give up."
-
Have you ever lost faith in coding while reviewing someone else’s code?
I did that mistake today! So annoying! I threw my laptop in the corner and picked up my phone to write this!2 -
Push a PR that fixes 7 bugs...
Feel like a pro :)
Trying to get the 2 required approvals for my PR on a Friday after lunch...
Feels like pulling teeth :( -
We've recently employed a new lead dev that seems to have a problem in that his solution is always the correct solution.
On a typical day, whenever I push code up for review via pull request, every single ticket I work on, he has something that has to change which doubles the amount of time of each ticket.
I'd be fine with this if the other 2 developers also think he's a bit of a headache in terms of his opinion but a lot of the time, there is always.. ALWAYS something that has to change because his method is better than mine.
For example, just now I pushed up some code that literally just adds in the user's email to the view which is already in the store for that action/effect anyway. I added one line of HTML.
He comments saying that I need to change the way it gets the email by doing a different request in the effect, to get the current user id, and from that match it against the email address, and THEN display it in the view.
This ticket took me 5 minutes. He's making me make it 30-60 minutes (to understand his requirement and implement it).
Is this normal? Am I over reacting?
Opinions please!7 -
the two code review personality types
review activity:
- dev A: requests code review, sets dev B and dev c (myself) as reviewers
- dev C comments: this review is marked with a complexity > 9000, touches > 20 files and has zero comments... also there's a lot of refactoring going on, making it hard for me to tell what the actual relevant changes are. can you please add more comments to this review?
- dev B (10 mins later): approved review6 -
There's a comment I left on a code review on Friday that I have absolutely zero recollection of even looking at the review. On the upside, apparently I did work in my sleep..? It's something that was discussed in today's stand-up...
-
I coded a simple Java programme when I was a beginner, today I reviewing it. I facepalm myself and thinking "I wish I know about 3 tier Architecture software development earlier" . Now I am looking at a one tier Architecture programme.
Which means UI components, Logic Components and Database components are all in ONE class and one method. Omg. Silly past self...4 -
My best code review was when my merge request was accepted in less than a minute after creation. It was simple but I expected more time on review and accept action.
-
I'm currently at a company where we have "performance reviews" every 2 weeks, and based on the outcome we get a percentage which then is used to calculate a performance bonus.
This is simply my manager (also a developer) who has his Excel spreadsheet, looking at tasks I did over the course of the past 2 weeks and almost nitpicking to find some fault. There is no code review or software demo to see what's been done either... I was there for the first 3 months and I don't think anyone had even open my code!
And when confronted, I get told that "You should also somehow be financially liable for the goings-on in the business", along with a 2 hour meeting to support that.
This is NOT how you motivate developers!
Apologies for the long rant...
</rant>7 -
Definition of code reviews in our company:
"Part of work that requires you to scroll to the bottom of the page and click accept button. Looking the changes is optional."4 -
Ticket waiting for code review for days. I have to rename methods.
Tickets goes again to code review. Waiting there again for days. Oops! there is something the code reviewer didn't see before!
Ticket goes to code review again, waiting for days there.
Boss comes to me telling it takes me too long to close tickets. -
How long do you think code reviews should be? Ours can run from anywhere between 10 minutes to 4 hours4
-
What is the best way to do a code review for a colleague's code, without coming out as a complete ass?16
-
What I absolutely hate the most of my workflow is to hand over my code for review to other developers.
I know it is important to prevent errors and to get feedback from them to improve, but I'm far from being self-confident and I'm afraid of showing others my work, regardless of the fact that nobody said anything mean about my work.3 -
That feel when you retire 152 files worth of dead code...
The diff was so big, it crashed our review tool! -
Genuine question, what was the most comments you've left in a single code review?
Just reviewed pull request submitted by a developer working for a contractor company and needed to leave 70 comments. Seventy.
Opened LinkedIn and saw a post from that same developer saying he left the contracting company an hour ago. I still can't believe it.14 -
Why comment on the same thing during code review??
I submitted a PR and had to make a design choice that propagated throughout the module i was working on.
During code review, my coworker commented on every...single...line that this change effected asking "why are we doing x here?" instead of just creating ONE SINGLE THREAD with this question for discussion. There were at least 10 review comments on github from their one review that said "why X?"
Is this normal? Ive only had a few programming jobs and this is the first time this has happened to me.
personally, when someone makes a choice like that, i just make a comment and save the rest of the review until that is addressed.5 -
Today a senior developer and a colleague started looking into my code reviews and started commenting best practices that were never used in the team.
Got my chance back at the senior developer's code when he raised a code review, which had none of the best practices.
Gave back a good set of review comments to him :D
Karma is a boomerang :)2 -
Boss(non-technical) complains we are behind the schedule he set without consulting the tech team, the complains we don't all(including BAs) don't do code review. CHOOSE!
*Note: I am not bashing code review*2 -
Code review moment that I hate
Me: This is a bad practice. You shouldn't do this
Developer: But it works
*Showed articles and examples why it's bad*
Developer: I see. But it works. Why should I change it?2 -
Nothing excites me more than a beautiful, simple and logical piece of code...
Sometimes I even feel like a creep with that obsession.
Am I the only one here?1 -
I work at a small company (less than 10 developers). We tried to do code review but all of our projects are "work for hire" for our clients and we always have deadlines. Code review step always had the least priority and whenever the deadline gets closer, we would stop doing code review all together.
Finally we are starting our own projects and we are planning on hiring more developers and interns. I think code review will have a higher priority now.1 -
If you're reviewing someone's code, do you run/test the code before reviewing the logic? Or do you review logic before running the code?4
-
Customer has design review for big project I’m working on, have to sit with someone who isn’t a developer and explain my code.
Cunts, cunts, cunts.
If you don’t understand or know about code maybe you shouldn’t be a code reviewer. -
How do i handle code review as a junior developer?
I understand its need and I don't want to skip it.
But I feel under confident after code review when my mistakes are pointed out. They always seem so trivial and wonder how did I even miss them. How should I handle such situations? And how do I improve from here?6 -
This is why code reviews are important.
Instead of loading a relevant dataset from the database once, the developer was querying the database for every field, every time the method interacted with it.
What should have been one call for 200k records ended up as 50+ calls for 200k records for every one of 300+ users.
The whole production application server was locked.2 -
What's the longest code review meeting you've ever had? We're about 3 hours into this code review so far, and it doesn't sound like we're going to be done anytime soon3
-
Best code review experience was when I was mentor in a bootcamp and I had to review code from scholars, they were surprised by how their code could be written in less lines.
-
New job. "Wtf" code.
I can see my performance review being hampered by the fact that I'm not intimate with this mess.2 -
Me on code review : You should extract the logic into a separate method for better readability. Colleague completely ignores it. What should I do?6
-
<rant>
Those nagging coworkers who first insist then plead followed by beg to approve a code review just before the deadline. Sigh ;-(
</rant> -
Having a meeting to decide, when to have other meetings...
Scrum, scrum of scrums, workstream, planning, pm ,design review, architecture review, Sprint review on and on....on and on on...why can't i simply code:(4 -
Coding is like handwriting.
Code review is about having a common understanding of the big picture and ensure be features follow the general architecture and process flow.
Code review is not about nitpicking on FUCKING TRAILING WHITEFUCKINGSPACES , lower case vs upper case SQL statements, extra empty lines AND EVERY FUCKING MINOR DETAIL you can imagine18 -
What do you guys think of code review? It was supposed to find potential mistakes in your code base, and share your knowledge with your co-workers, right?
In fact I have very bad experiences with code review, not just with 1 company, but quite a few. Code review process always comes to something like this:
Reviewer: Hey, I don't like your solution A because of disadvantages A1. You should implement solution B, because of advantage B1 blah blah...
Me: Yes, it's true that solution B provides advantage B1, but at the same time introduces much more complexity to the code base than necessary, and has disadvantage B2. I am aware that solution A has disadvantage A1 but it is justifiable and easier to overcome than B2 imo. In fact, solution A also provides advantage A2 that you might not know...
Reviewer: No, you HAVE TO implement B because of advantage B1 blah blah *repeating why B1 is awesome again*
I feel like it's just people trying to shove their preferences down my throat. Either code review is useless, or the companies I work for do something very very wrong in code review. Anyway, it's really discouraging me fron participating in team discussions.5 -
We only recently started and we can really see the benefits of code review.
It motivates you to follow the standards, writing good quality code and using variable/function names that makes sense. Especially that you know someone is going to read through it.1 -
Today I read a book and it talks about giving feedback on others; as a developer we tend to give feedback and review others code almost everyday. When I write the code I always put a lot of effort on making it look at prefect but I found I am having trouble on receiving others criticising and comments being judgemental. Instead if we make it as a "draft" rather than "final", that will make you becoming more open towards others opinion. I find this is quite true and I hope this could benefit developers who have similar attitude :)2
-
Picked up an issue to contribute to OSS for a community version of a major enterprise software. Did the changes, submitted a pull request. Someone reviewed it, asked for some changes, which i did and pushed the changes.
Then after some discussion with the guys working there, we thought of making some changes to the UI. Step in the company UI guy, he makes some changes, i merge his branch into mine and submit a new pull request.
Now, a new guy comes in to review the code, who has a problem with every change THEIR UI Guy did, and negates everything the first reviewer said, and asks me to do the changes, and boy was I pissed!!
But I did the changes, updated the PR, then the first reviewer comes in again, and suggests some more changes, most of them are for the code, THEIR UI Guydid!! Fucking psychopaths!! Never had i seen such paranoid people in my life!! Educate your fucking team first!!
I one again started with the changes but left mid way!! Now, even if i want to, will not update the PR!! FUCK YOU!!3 -
If there's one thing I hate about devs is definitely when they get too emotional about the reviews they receive.
Doing a thorough review always takes significant amount of time and energy. It's about ensuring high quality of code, about functionality and best practices, ... It's also about learning: I learn from the changes being reviewed while at the same time I also try to teach the author as much as possible, giving down to earth opinions.
It's never (or at least should never be) about attacking the author. There really is no reason why someone would spend all this time getting overly personal.
I used to start my responses with (lousy) apologies for being "harsh", but stopped doing this now that my team understands all of this. It also helped asking them to do the same with my changes. The look in their eyes when they find something is simply invaluable :).1 -
Code got submitted for review... Syntax error.
Like wtf, your IDE even tells you about syntax errors... not to mention the failing build 😤 -
It is ok to fail and commit mistakes, that's part of the game, specially for beginner devs. Just avoid failing alone, the most you can!
I mean:
- Ask people to review your code before pushing to the source repository.
- If you are not sure how to do, ask.
- Never work in production environments without supervision. Pair with someone.
- Have a desk mate for rubberducking (https://en.wikipedia.org/wiki/...) and blame it in case you need -
so client asked to come on Saturday for code review for 3 hours and i went . then after 3 hours he asked us to sit for littler bit more. when we said we had plans already , he asked to come on sunday . is client legally or ethically right to ask us to come on weekends so that he can review the code .6
-
I'm slowly realizing how much goofy code I put in my branch and overlooked. This code review is going to be interesting...
Some examples:
import plots as lel
<h4 id="title">Crunchatize Me, Captain! </h4>
go.Scattergeo(name="cheese", ...)
webster = { ... }
The commit messages are even worse.
- 'horizontalize' link list
- very messily hack in <feature>
- partially refactor some of the awful code from previous
- Remove one annoying space
- make background color less annoying
- remove seemingly useless property
- minor fix
- Apparently it's possible to center a DIV. Who knew?
- Made some cool bar graphs
And then there's just a bunch of reverts.2 -
What do you do when pull requests are enforced, but everyone is so fucking apathetic about reviewing code?13
-
I packed double on mate today. Hopefully I'll survive the code review today. How I love fridays...rant shut up weak body of mine fridays are actually worse than mondays oh god of caffeine let me get through this3
-
Doing a code review (Of code written by another person...different code styles and weird methods can really get you a gray hair or two :D)2
-
Writing my first code review. Even though it really is a nice review and I'm happy with the solution code, I still somehow feel like an asshole for each critique I make. Maybe it's unavoidable with code reviews / pull requests?3
-
Not before long, I guess we may have to put up with bots during code review...😂
Meet the Bots That Review and Write Snippets of Facebook's Code
https://spectrum.ieee.org/tech-talk... -
Why? As a senior, you won't give some time to review my code, will let me merge my code to a branch, then blame us when it will produce the bug in production? why? 😐 Won't even arrange a code review/knowledge sharing session so that juniors can learn at least something. Even you won't encourage us write test cases. If seniors don't follow, are the juniors to blame? 🙂3
-
So when writing golang code, you can put alias into the imported library
And one of interns in my office include include our company's library called "helper/session", and alias it with 'hSession'
Now all the code which use that library has to look like "hSession.ErrUnexpected", "hSession.NewSession"
Well, i think i've played too much VN and anime, but still i'm having a good laugh when doing the code review -
Does anyone else get frustrated when your co-worker goes behind you and changes the name of a particular variable? Changing the word "repo" to "repository" does not clarify a fucking thing! You're not going to confuse it with something else. I've never once seen the word and thought "Damn, that guy meant reposition and I just fucked everything up." It would be one thing if our lead Dev told me to not use the word, but he could not care less.
Am I in the wrong?1 -
Men fo real! I dont rant so much because I think its a negative attitude but let me do it anyway! Listen. My boss boss told me to create a dynamic drop which I did. A backend request then display it on the frontend which is easy, then on code review he ask why do we need this error handling. Bruh as soon as I heard that question, I got covid. Bitch we do need that error handling because if theres error on requests it will set to default options, but I didn’t say anything tho. I just ask what will happen if there’s an error?, he said I don’t think a simple request will respond error if you did it right. Then I agreed and remove the code. Hot damn! Mind you guys. When they started the app there are no test code. 0, nada, nothing inside the spec folder.7
-
I had been working to make API call Asynchronous..
And now complexity is too High..
Yet i am happy that Code working as expected..! -
Oh the joy of multi-site working and design reviews in bigger corporations...
I try to propose if we could do it on-line with BitBucket commenting etc. Just put your comments there, we discuss it there, each in our own time, and get things closed.
But no. It's nicer to arrange 2-3h conf calls. So that we can really discuss items (and the reviewers don't have to do anything before the call). Nothing can be done beforehand. And the reviewers get to comment not only on design matters, but on system level things too. Like "I wonder if this would be better in place X". Well sure, maybe, but that's system level decision and would require architects etc. And all that work was done 2 years ago, we're supposed to now just check the source code (which you guys wanted me to change).
Ok, so I will arrange a conf call. Our time zones are not the same, so one guy is coming to the office when another is almost leaving. One wants to have Wednesdays meeting free. One has lunch at 11, another at 13. For fucks sake. Some guys have filled their calendar with meetings, most of them which they will not attend anyway, but Outlook shows them as "reserved".
So I spend my day trying to find a free spot that everyone could join. Half of the guys won't read the code and won't give any comments, but still need to be there. And then there are those comments saying "I'd like this variable name to be different" and "it would be cleaner if this was done like I do". Same people produce unreadable mess themselves, but somehow always manage to dodge all reviews of their own stuff. -
Best way to measure code quality is by using wtfps method. Less "what the fu#*s per second" you get, better the code is.
-
Just askin:
If you have a method which returns a value from an array. What do you prefer in a case when the item is not found?
A)Throw an exception
B)return null
C)return a special value like a null object or some primitive type edge value like Integer.MIN_VALUE14 -
Finally after spending an hour and my lunch break on getting rid of 120+ char lines and white spaces.. 😇✌🏻️1
-
Does code review plays a big role when developing? Although it takes time? and also what's the average time when doing it?11
-
Why include a linter if you're just going to ignore it!
I just "inherited" an angular app from a year ago for a project that was put on hold, and after opening it in VS Code practically every TS file went red. Almost every rule in the config was not followed. Might as well have just disabled the darn thing?
The original developer is MIA so I can't contact him and ask him why either.1 -
coworker: "Did you see my code review?"
me: "Yeah, I haven't gotten to it yet, I'm sorry."
coworker starts sobbing.
me: questioning existence. -
That moment when you have to send a 3 line pull request for one of the newer guys to review immediately, and he gets super anal retentive over the order in which arguments appear2
-
Got a code exercise. A small cli nodejs tool. Could someone do a code review? https://github.com/kenpeter/mb1
-
Together with colleagues from University of Zurich I am conducting a survey about skills for code review! With this survey, we aim to investigate skills that reviewers need to perform an efficient code review. Our main goal is to improve developers' code review practices. Therefore, we are looking for developers with experience as code reviewers. This questionnaire takes 15-20 minutes and consists of 21 questions.
https://uzh.ch/zi/cl/...
Thank you for your help!4 -
I don't know if I'm terrible, or if this will sound familiar to anyone. I rushed so much of this project. That's not a good excuse for what's happening, but, speaking about it with a newly converted coder who is a good friend of mine, let him be called F:"
F: I'm so bored I'm going through my script and making a few subs for some repetitive code. I saved 90 lines today.
Me: Bored you say... debating what sort of code of mine to send you for you to ... review.
Because, the reality is, if I dont finish certain features by May, shit will hit the fan lol So I am considering asking for a boilerplate NDA and a few extra bucks from client.nickname, to bring on testers and/or UI guys and/or database guys.
But you seemed to be doing alot lately, so I was thinking, I would deal with fiverr and freelancer.com first
F: I dunno what use id be by May but I'll always look at stuff
Me: A ton. You could literally review any code in any language youre learning. Your review code be: address/models.py class Address 1. TODO for validating formatted address 2. Why is formatted address declared twice?
To which my response would be Fuck thats right and Zomg really
And if I knew about this... last week.. I'd be hours ahead of schedule and not have just forgotten why I needed to fix address
F: Lol" -
SeniorDev(in code review): Yeah, I know this is wrong but I will look into it later
Me: Can you please mention the ticket you have created to look into it later
[JuniorDev gives me a high five for sticking to our coding principles. No sweeping under the rug! Felt awesome.] -
Code review time.
"How come this line has been removed? PEP 8 likes to have two lines between imports and the first bit of code"
What I replied: Thanks. I'll put it back.
What I wanted to reply: Go fuck yourself you anal moron, who the fuck gives a shit about bollocks like that. We got fucking proper work to do, so get the fuck over yourself, let the fucking PEP shit lie, and make some fucking USEFUL comments.5 -
when i work, i code. whe i sleep, i code. When i talk to friends, i talk about code. I figured out that its going to be a hell of a code review when I'm gone.1
-
Is there anywhere that you can get your open source code brutally reviewed?
As the (almost) sole developer on a project (and only just entering the world of professional software development), I have no idea if the stuff I've written is good, bad, or just plain disgusting, and obviously the only way to improve is to be aware of the mistakes you made3 -
in agile methodology, have a daily session of 3 hours code review in which he will understand my code.. 👿
-
Code Review is one fucking awesome stunt.
- Take a long sommersault flip.
- Land on your face or your ass, it doesn't matter.
TBH, lot of learning insights during the review. -
We were developing a new feature and on the code complete day, I had to take leave. While code review one of the devs changed my code without proper analysis.
End Result, half the things cannot be tested until that piece of code is fixed1 -
It feels like there is a rule of the internet that any code snippet visible is immediately subject to review by the comments.1
-
Recently I've finally finished my first game in Unity3D <3 But I'm self-taught and it's probably not really well-made. I'd love to show code to someone with real experience but I don't have any friends in game dev -.-
Does anyone know where I could get some kind of code review (for free would be great, since I won't earn a penny from this game)?
Shameless plug for anyone interested:
https://play.google.com/store/apps/...1 -
People that approve pull requests without looking at them!
No tests or so bad they would do more use by not existing, typos, the code follows none of the design practices and the code obviously will not compile and thereby breaks builds in trunk for everyone.
Because of course they only asked one person to review it and then merged it immediately. -
getting tons of fucking merge conflicts on rebasing remote branch with current master because that mockerfucking code review is still pending
-
Afternoon spend arguing about small cosmetic changes in code-review effectivelly delaying the release. Feel so productive and understand why everything takes us so long. But looking forward to next week so time sunked in code-review will be larger than the development time.3
-
When I get a code review by my self-proclaimed expert colleague suggesting a change that ends up breaking the feature, I just implement that spaghetti code and let the testers know I'm not to blame.
-
The smaller the code changes the longer the name of PR for the code.
And even more review changes mentioned by the reviewer... -
Any android native devs here? I need a code review, any feedback appreciated https://github.com/l2dev/...8
-
Anyone free and willing to help me with a react code review?
I’m stuck somewhere and not sure where i went wrong12 -
XXXX programming language does not scale. Nope, it is your code. Review your code and improve it! Don't add more hardware to improve performance, that solution just covers the problem. Review, review and review
-
How does your organisation and team balance PR comments demanding changes and dev time?
Here, while fixing PR comments we sometimes end up wasting as much time as we took in actually developing the feature... As a result, almost every major user story overshoots the estimation and almost every sprint gets delayed.
Yes, to each his own; but talking in general, why do you think this time wasting happens?
Do you think that happens because some of us are not as experienced as the others, the existing code not being up to the mark giving a bad example, or just a skewed review process?2 -
Sometimes I'll block a code submission with the words security vulnerability", then go have a 10 minute break to see if the others can spot it on their own.
-
Sometimes I don't do a proper review because I don't want to seem very nitpicking. However, now I have to maintain the code that was merged and it has some stuff which is not correct. Now I have to fix it.1
-
Today I submitted to code review the first iteration of a microservice done with Ramda and flow by request of my collegues. This is the first time they look at anything similar to functional code or typed js, and only one of them took the time to actually do a review.
I really like having my code reviewed and reviewing others', but please don't pester me to make a PR for a microservice you'll never look only to bail off as soon as you see something new that scares you. Buckle up and learn new stuff! -
Is this a justified code review comment or a bully?
Code reviews are weakness of this industry which has the potential to attract bullies. Abuse of the comment box in a pull request and bombarding the employee with hundreds of comments can cause stress, frustration, burnout and finally resignation and costs of fulfillment for the organization. While companies should find and stop bullying in the work place, what kind of code review comment is considered a bully and why? Any of below traits can mean you are dealing with a bully:
1. Claims the code needs to be changed but doesn't say how. So no matter how many times you change your code, he can repeat the same comment: "Your code is still bad due to blah blah and it needs to be changed".
2. Provides how the code should be changed, but the change doesn't add up to quality, security, performance, readability, etc. i.e. "Why did you use a for loop here? Use a while loop instead". Or "Why did you write it using three classes A, B and C? Instead write it using 4 classes D, E, F and G which does blah blah". In the later case, not following the review comment, you won't get approval. Following the comment means you need to rewrite your whole code. After which, you might again receive more comments to change other parts of your code!
3. Claims the requested change is due to standards but claimed standard does exist anywhere. Internet, company wiki, university course books, anywhere. In more severe cases of psychopathy, the bullying person refers you to a link which hours later turned out to be written by himself! Have fun describing what has happened to your manager or team leader... .
4. Asks the code to be changed in a way that supposedly is closer to standard or of better quality, security, performance, etc. But the proposed way will not work and is the main reason you didn't do that in the first place. So you start arguing forever in the comment box over why his method won't work!
If you cannot see any of the above traits, then keep calm, take a breath, fix your code. Otherwise you might be victim of a bully.3