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 comment"
-
"You gave us bad code! We ran it and now production is DOWN! Join this bridgeline now and help us fix this!"
So, as the author of the code in question, I join the bridge... And what happens next, I will simply never forget.
First, a little backstory... Another team within our company needed some vendor client software installed and maintained across the enterprise. Multiple OSes (Linux, AIX, Solaris, HPUX, etc.), so packaging and consistent update methods were a a challenge. I wrote an entire set of utilities to install, update and generally maintain the software; intending all the time that this other team would eventually own the process and code. With this in mind, I wrote extensive documentation, and conducted a formal turnover / training season with the other team.
So, fast forward to when the other team now owns my code, has been trained on how to use it, including (perhaps most importantly) how to send out updates when the vendor released upgrades to the agent software.
Now, this other team had the responsibility of releasing their first update since I gave them the process. Very simple upgrade process, already fully automated. What could have gone so horribly wrong? Did something the vendor supplied break their client?
I asked for the log files from the upgrade process. They sent them, and they looked... wrong. Very, very wrong.
Did you run the code I gave you to do this update?
"Yes, your code is broken - fix it! Production is down! Rabble, rabble, rabble!"
So, I go into our code management tool and review the _actual_ script they ran. Sure enough, it is my code... But something is very wrong.
More than 2/3rds of my code... has been commented out. The code is "there"... but has been commented out so it is not being executed. WT-actual-F?!
I question this on the bridge line. Silence. I insist someone explain what is going on. Is this a joke? Is this some kind of work version of candid camera?
Finally someone breaks the silence and explains.
And this, my friends, is the part I will never forget.
"We wanted to look through your code before we ran the update. When we looked at it, there was some stuff we didn't understand, so we commented that stuff out."
You... you didn't... understand... my some of the code... so you... you didn't ask me about it... you didn't try to actually figure out what it did... you... commented it OUT?!
"Right, we figured it was better to only run the parts we understood... But now we ran it and everything is broken and you need to fix your code."
I cannot repeat the things I said next, even here on devRant. Let's just say that call did not go well.
So, lesson learned? If you don't know what some code does? Just comment that shit out. Then blame the original author when it doesn't work.
You just cannot make this kind of stuff up.105 -
My "Coding Standards" for my dev team
1.) Every developer thinks or have thought their shit don't stink. If you think you have the best code, submit it to your peers for review. The results may surprise you.
2.) It doesn't matter if you've been working here for a day or ten years. Everyone's input is valuable. I don't care if you're the best damn programmer. If you ever pull rank or seniority on someone who is trying to help, even if it isn't necessarily valid or helpful, please have your resume ready to work elsewhere.
3.) Every language is great and every language sucks in their own ways. We don't have time for a measuring contest. The only time a language debate should arise is for the goal of finding the right one for the project at hand.
4.) Comment your code. We don't have time to investigate what the structure and purpose of your code is when we need to extend upon it.
5.) If you use someone else's work, give them the credit in your comments. Plagiarism will not be tolerated.
6.) If you use flash, you will be taken out back and shot. If you survive, you will be shot again.
7.) If you load jQuery for the sole purpose of writing a simple function, #6 applies.
8.) Unless it is an actual picture, there is little to no reason for not utilizing CSS. That's what it's there for.
9.) We don't support any version of Internet Explorer and Edge other than the latest versions, and only layout/alignment fixes will be bothered with.
10.) If you are struggling with a task, reach out. While you should be able to work independently, it doesn't make sense to waste your time and everyone else's to not seek assistance when needed.
11.) I'm serious about #6 and #7. Don't do it.48 -
You know what?
Young cocky React devs can suck my old fuckin LAMP and Objective-C balls.
Got a new freelance job and got brought in to triage a React Native iOS/Android app. Lead dev's first comment to me is: "Bro, have you ever used React Native".
To which I had to reply to save my honor publicly, "No, but I have like 8 years with Objective-C and 3 years with Swift, and 3 years with Node, so I maybe I'll still be able help. Sometimes it just helps to have a fresh set of eyes."
"Well, nobody but me can work on this code."
And that, as it turned out was almost true.
After going back and forth with our PM and this dev I finally get his code base.
"Just run "npm install" he says".
Like no fuckin shit junior... lets see if that will actually work.
Node 14... nope whole project dies.
Node 12 LTS... nope whole project dies.
Install all of react native globally because fuck it, try again... still dies.
Node 10 LTS... project installs but still won't run or build complaining about some conflict with React Native libraries and Cocoa pods.
Go back to my PM... "Um, this project won't work on any version of Node newer than about 5 years old... and even if it did it still won't build, and even if it would build it still runs like shit. And even if we fix all of that Apple might still tell us to fuck off because it's React Native.
Spend like a week in npm and node hell just trying to fucking hand install enough dependencies to unfuck this turds project.
All the while the original dev is still trying TO FIX HIS OWN FUCKING CODE while also being a cocky ass the entire time. Now, I can appreciate a cocky dev... I was horrendously cocky in my younger days and have only gotten marginally better with age. But if you're gonna be cocky, you also have to be good at it. And this guy was not.
Lo, we're not done. OG Dev comes down with "Corona Virus"... I put this in quotes because the dude ends up drawing out his "virus" for over 4 months before finally putting us in touch with "another dev team he sometimes uses".
Next, me and my PM get on a MS Teams call with this Indian house. No problems there, I've worked with the Indians before... but... these are guys are not good. They're talking about how they've already built the iOS build... but then I ask them what they did to sort out the ReactNative/Cocoa Pods conflict and they have no idea what I'm talking about.
Why?
Well, one of these suckers sends a link to some repo and I find out why. When he sends the link it exposes his email...
This Indian dude's emails was our-devs-name@gmail.com...
We'd been played.
Company sued the shit out of the OG dev and the Indian company he was selling off his work to.
I rewrote the app in Swift.
So, lets review... the React dev fucked up his own project so bad even he couldn't fix it... had to get a team of Indians to help who also couldn't fix it... was still a dickhead to me when I couldn't fix it... and in the end it was all so broken we had to just do a rewrite.
None of you get npm. None of you get React. None of you get that doing the web the way Mark Zucherberg does it just makes you a choad locked into that ecosystem. None of you can fix your own damn projects when one of the 6,000 dependency developers pushes breaking changes. None of you ever even bother with "npm audit fix" because if security was a concern you'd be using a server side language for fucking server side programming like a grown up.
So, next time a senior dev with 20 years exp. gets brought in to help triage a project that you yourself fucked up... Remember that the new thing you know and think makes you cool? It's not new and it's not cool. It's just JavaScript on the server so you script kiddies never have to learn anything but JavaScript... which makes you inarguably worse programmers.
And, MF, I was literally writing javascript while you were sucking your mommas titties so just chill... this shit ain't new and I've got a dozen of my own Node daemons running right now... difference is?
Mine are still working.34 -
Dev: what do I call this file ?
Me: just name it something meaningful so other dev's know what it is
Two days pass
Me: time to do code review .. oh look a new file ..
Git comment : new file for sax parsing , architecture gave the ok.
File name : SomethingMeaningful.java11 -
After I submitted a code review:
Coworker: What did you mean with this comment?
Me: **translating the comment to Portuguese** Your Footer component isn't rendering any footer element.
Coworker: **blank stare** what?
Me: There is no footer tag here. **points to Footer component**
Coworker: **computing... found approximate result** I'm rendering the Footer here. **shows me where the Footer component is being rendered**
Me: **internal facepalm** Yes, I know, but I'm not talking about that. I'm saying that inside the Footer component you should be rendering a footer element.
Coworker: **segmentation fault** what?
And then I had to explain that there is an HTML footer element. To a mid level frontend developer (or so they say).
HTML is not only divs, for fuck sake.26 -
I've written a lot of bad code, seen a lot, but attached is the most recent 'worst' I've seen.
What makes the situation worse/funny is:
1. The developer's code comment.
2. Check-in passed a code review.
3. The 'legacy code' was written last week.29 -
@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 -
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
-
Code review time:
Hey Rudy, can you approve my PR? ??? Shouldn't it be can you review my PR?(thought to myself)
Anyway, as a new practice, we(royal we) do not approve PRs with js files. If we touch one, we convert it to typescript as part of a ramp up to a migration that never seems to get here. But I digress.
I look at the laziest conversion in history.
Looked like
Import 'something';
Import 'somethingElse';
Import 'anotherSomething';
export class SomeClass {
public prop1: any;
public prop2: any;
public prop3: any;
public doWork(param: any){
let someValue = param;
// you get the idea
return someValue;
}
}
Anyway, I question if all the properties need to be visible outside of the class since everything was public.
Then if the dev could go and use type safety.
Then asked why not define the return type for the method since it would make it easier for others to consume.
Since parts of the app are still in js, I asked that they check that that the value passed in was valid(no compilation error, obviously).
Also to use = () => {} to make sure "this" is really this.
I also pointed out the import problem, but anyway.
I then see the his team leader approve the PR and then tell me that I'm being too hard on his devs. ????
Do we need to finish every PR comment with "pretty please" now?
These are grown men and women, and yet, it feels oddly like kindergarten.
I've written code in the past that wasn't pretty and I received "WTF?" as a PR comment. I then realized I ate sh*t on that line of code, corrected it and pushed the code. Then we went to Starbucks.
I'm not that old(35), but these young devs need to learn that COMPILERS DO NOT CARE ABOUT YOUR FEELINGS!!!!!
Ahhhhhh
Much better.
Thanks for the platform.8 -
Working on a project with 2 other students. One of them makes a C# "super class" with 50 fields, and manually creates getters and setters for each and every one. Then he proceeds to write a constructor that accepts 50 parameters, because why not.
I comment on the git commit, telling him that he can just write " get; set; " in C# and that he should model the problem in smaller, more manageable classes ( this class had 270 lines and did everything from displaying data to calculating stuff). Tried to explain to him that OOP works kind of differently from how he did it.
....
His answer: "Yeah, I don't really care. If it works once, it's okay for me".
This after the most beautiful code review I have ever done...
Fml8 -
New developers(5-6 years experience) these days are so pathetic. They dont have any sense of code review. All they want is to put their opinion out without giving any thought.
I had a PR for review today which contains mock specification to match a regular expression and return the corresponding response
The regular expression I put was
104000(02|06|20|48)
Now, this guy comes and puts a comment that we could "simplify" as 104000\d{2}
I replied, the ending digits are not contiguous. The specific pair of digits have to match for these mocks.
Then this guy replied, then we could simplify as 104(0{4}(2|6)l0{3}(20|48)).
I said, I cannot understand how that is simplification. Why do we need such a complex regex to match something very straight forward.
And the guy replied, we should be writing proper regexes, otherwise we could just specify everything explicitly.
I was like WTF man. You try deciphering this next week without taking at least a minute to know which values are matched.
Anyhow, another senior person approved my PR, and I merged it.12 -
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 -
Self documenting code is a fucking myth you bloody sheep.
Write “self documenting code” then add a fucking comment or two explaining why the fuck the code deserves should be there because nobody can see what the fuck it is doing or understands how the whole collection of microservices works. I’m sick of spaghetti code bullshit full of accidental redundancy because it is impossible for anyone to realize why something is there at a glance.
I renamed different “Contract” classes today by adding numbers before code review.
Contract
Contract1
Contract2
Contract3
All of these classes are supposed to be the same but somehow they aren’t and you self documenting dumbasses missed it. Don’t gripe about the numbered classes in the repo… fix the fucking code and collapse the classes so we don’t have four sections of code describing the same fucking structure from a http get with different interfaces because four people couldn’t read the whole like some fucking computer.11 -
I'm going on vacation next week, and all I need to do before then is finish up my three tickets. Two of them are done save a code review comment that amounts to combining two migrations -- 30 seconds of work. The other amounts to some research, then including some new images and passing it off to QA.
I finish the migrations, and run the fast migration script -- should take 10 minutes. I come back half an hour later, and it's sitting there, frozen. Whatever; I'll kill it and start it again. Failure: database doesn't exist. whatever, `mysql` `create database misery;` rerun. Frozen. FINE. I'll do the proper, longer script. Recreate the db, run the script.... STILL GODDAMN FREEZING.
WHATEVER.
Research time.
I switch branches, follow the code, and look for any reference to the images, asset directory, anything. There are none. I analyze the data we're sending to the third party (Apple); no references there either, yet they appear on-device. I scour the code for references for hours; none except for one ref in google-specific code. I grep every file in the entire codebase for any reference (another half hour) and find only that one ref. I give up. It works, somehow, and the how doesn't matter. I can just replace the images and all should be well. If it isn't, it will be super obvious during QA.
So... I'll just bug product for the new images, add them, and push. No need to run specs if all that's changed is some assets. I ask the lead product goon, and .... Slack shits the bed. The outage lasts for two hours and change.
Meanwhile, I'm still trying to run db migrations. shit keeps hanging.
Slack eventually comes back, and ... Mr. Product is long gone. fine, it's late, and I can't blame him for leaving for the night. I'll just do it tomorrow.
I make a drink. and another.
hard horchata is amazing. Sheelin white chocolate is amazing. Rum and Kahlua and milk is kind of amazing too. I'm on an alcoholic milk kick; sue me.
I randomly decide to switch branches and start the migration script again, because why not? I'm not doing anything else anyway. and while I'm at it, I randomly Slack again.
Hey, Product dude messaged me. He's totally confused as to what i want, and says "All I created was {exact thing i fucking asked for}". sfjaskfj. He asks for the current images so he can "noodle" on it and ofc realize that they're the same fucking things, and that all he needs to provide is the new "hero" banner. Just like I asked him for. whatever. I comply and send him the archive. he's offline for the night, and won't have the images "compiled" until tomorrow anyway. Back to drinking.
But before then, what about that migration I started? I check on it. it's fucking frozen. Because of course it fucking is.
I HAD FIFTEEN MINUTES OF FUCKING WORK TODAY, AND I WOULD BE DONE FOR NEARLY THREE FUCKING WEEKS.
UGH!6 -
Fucking retards littering my code review.
I had posted some important questions in my comment (essential in order for the code review to move forward) and this asshole posted some stupid out of context meta crap comments no way related to the subject of the review pushing my questions out to the point that a lot of scrolling is needed to see them. Now I need to get everyone’s attention back to the point that matters. Why does this happen to me ? This actually happens quite a lot with other forums as well. Trash mouths going off topic.6 -
I'm a contractor at a product company and today I had the pleasure of working with some jQuery.
A function needed to be called before another function, hard work right?
So I moved the call to the function 3 rows higher, checked it in, set the task as ready for test and started to look for other tasks.
Within a couple of minutes I get a direct message from another dev, let's call him Steve.
Steve wanted me to set the task to ready for code review instead of test, so I did just that and tried to move on.
Some minute or two later Steve contacts me again:
"It would be great if you'd move the comment so it'd be over the call to the function"
Well, I'm not one of those who likes comments... If you need a comment, it's probably not good/readable code. In some cases sure, it might be a complex block coming up.
Sorry, lost my train of thought.
I answered Steve : "Are you sure, I could just remove it instead?"
(for readability S will be Steve and M will be me)
S: Well, it's always good to have comments
M: In this case I think it will be alright.
S: But it's nice to see what the function is doing.
M: I'll do it if you really want me to.
S: It's better to have the comment than to not have it and needing it.
M: Okay then
The name of the function : LoadOrganizationTree()
And this is the comment :
//Load organization tree6 -
I was asked to fix a critical issue which had high visibility among the higher ups and were blocking QA from testing.
My dev lead (who was more like a dev manager) was having one of his insecure moments of “I need to get credit for helping fix this”, probably because he steals the oxygen from those who actually deserve to be alive and he knows he should be fired, slowly...over a BBQ.
For the next few days, I was bombarded with requests for status updates. Idea after idea of what I could do to fix the issue was hurled at me when all I needed was time to make the fix.
Dev Lead: “Dev X says he knows what the problem is and it’s a simple code fix and should be quick.” (Dev X is in the room as well)
Me: “Tell me, have you actually looked into the issue? Then you know that there are several race conditions causing this issue and the error only manifests itself during a Jenkins build and not locally. In order to know if you’ve fixed it, you have to run the Jenkins job each time which is a lengthy process.”
Dev X: “I don’t know how to access Jenkins.”
And so it continued. Just so you know, I’ve worked at controlling my anger over the years, usually triggered by asinine comments and decisions. I trained for many years with Buddhist monks atop remote mountain ranges, meditated for days under waterfalls, contemplated life in solitude as I crossed the desert, and spent many phone calls talking to Microsoft enterprise support while smiling.
But the next day, I lost my shit.
I had been working out quite a bit too so I could have probably flipped around ten large tables before I got tired. And I’m talking long tables you’d need two people to move.
For context, unresolved comments in our pull request process block the ability to merge. My code was ready and I had two other devs review and approve my code already, but my dev lead, who has never seen the code base, gave up trying to learn how to build the app, and hasn’t coded in years, decided to comment on my pull request that upper management has been waiting on and that he himself has been hounding me about.
Two stood out to me. I read them slowly.
“I think you should name this unit test better” (That unit test existed before my PR)
“This function was deleted and moved to this other file, just so people know”
A devil greeted me when I entered hell. He was quite understanding. It turns out he was also a dev.3 -
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. -
I have a guy sitting next to me in class. We were working on the same project. It's about rewriting a functioning mergesort algorithm in C and doing a presentation about that topic.
Now... the thing is that I was ill on that specific day when we got that project assigned. And he didn't tell me it either. I asked the whole class.
They just said that there was nothing special about that day. These fuckers.
Anyway...
Thé following week we had the same lesson again. Actually there were more than both of us. We were a group of 5 dudes.
3 of them barely have anything to do with programming at all. They just learn for the exams and have bad grades in programming.
Luckily, they already wrote the functioning sorting algorithm.
Since that is the case, I chose to review it to get deeper into that topic.
There were comments in English (we live in Germany) and these comments were written in a different style. My classmates would never comment in such a way.
It was a modified version copied from the internet. The whole source code.
The variables had names like j,k,b,u and so on. It was perfectly obfuscated.
Yesterday, I wasn't at college either.
I had to show up to a given time at a government bureau. They have been working on that project that day. So, I decided to ask them via a messenger, if they can give me the newest presentation files after 1 pm.
They said that they barely have anything to present. They would like to improvise they said.
"Fuck you all" I thought.
I'm done with these fucking illiterate humans.
I hope they all die in hell with satan having a ride on them. Stabbing them from behind right into their assholes and eating their ball sacks (if they have any).
Today is the presentation.
That's when I decided not to drive there during these specific lessons.1 -
Oh gee whiz fellas. I lived through my nightmare. Recently too.
(Multiple rants over last few months are merged in this one. Couldn't rant earlier because my login didn't work.)
I joined a new shithole recently.
It was a huge change because my whole tech stack changed, and on top of that the application domain was new too.
Boss: ho hey newbie, here take this task which is a core service redesign and implementation and finish it in two weeks because it has to be in production for a client.
Normally I'd be able to provide a reasonable analysis and estimate. But being new and unaware of how things work here, I just said 'cool, I'll try my best.' (I was aware that it was a big undertaking but didn't realize the scope and the alarming lack of support I'd get and the bullshit egos I'd have to deal with)
Like a mad man I worked 17+ hours a day with barely a day off every week and changed and produced a lot of code, most of it of decent quality.
Deadline came and went by. Got extended because it was impossible (and fake).
All the time my manager is continuously building pressure on me. When I asked questions I never got any direct/clear answers. On asking for help, I'd get an elaborate word vomit of what was already known/visible. Yet I finally managed to have an implementation ready.
Reviewer: You haven't added parameter comments on your functions and there aren't enough comments in code. We follow standards. Clean code and whatnot. Care for the craft verbal diarrhea.
Boss: Ho hey anux, do you think we'll be able to push the code to production?
Me: Nope. We care for the craft and have standards. We need to add redundant comments to self documented code first, because that is of utmost importance as Nuthead reviewer explained.
(what I wish I had said)
What I actually said: No, code is not reviewed yet.
And despite examples of functions which were not documented (which were written by the reviewer nut), I added 6-7 lines of comments for my single line functions describing how e.g. Sum takes two input integers and returns their sum and asked for a review again.
Reviewer: See this comment is better written as this same-meaning-but-slightly-longer way. Can we please add full stops everywhere even though they were not there to begin with? Can we please not follow this pattern and instead promote our anti-pattern? Thanks.
Me: Changed the comments. Added full stops. Here's a link for why this anti-pattern is bad.
Reviewer: you have written such beautiful code with such little gems. Brilliant. It's great to see how my mentoring has honed your skills.
.
.
.
I swear I would have broken a CRT on his stupid face if we weren't working remotely (and if I had a CRT).
It infuriates me how the solution to every problem with this guy is 'add a comment'.
What enrages me more is that I actually thought I could learn from this guy (in the beginning). My self doubt just made me burnout for little in return.
Thankfully this living nightmare will soon be over.rant fuck you shitty reviewer micromanagement by micrococks wk279 living nightmare fml glassdoor reviews don't lie9 -
*senior dev comes over to my desk*
"Your code is so wrong I didn't even bother to comment on the review." He proceeds to lecture me about why the code that I modified (a 2 year old file I included a tiny helper module in) needs to be rewritten. -
So I wrote a python code and was waiting for +1 on code review and I needed to merge it fast. That shit of a reviewer took his time to finally NOT give a +1 with comment, "if statement has no else part". OF COURSE IT DOESN'T HAVE ELSE PART. I DON'T NEED A ELSE PART. But to give him the benefit of doubt, I'd like to ask devRant community if they believe all ifs should have elses.14
-
i was hired to join a team of old devs (40+) in an unnamed European country "yay goodbye 3rd world it's time to enjoy the quality of life" assist with enhancing already existing software and creating new solutions.
prior to my arrival most things were slow and super buggy, looking at the code base it shouldn't be a surprise, amateur hour everyone, logic implemented that is not needed, comment driven development, last time code review was done back in 1996. lots of anti patterns.
i swear there is a for loop that does nothing but it loops through a 100+ elements list, trunk based development with tfs since git is "not really needed"
test projects are not there.
>enter me an educated fool, with genuine passion for the craft and somehow a decent amount of knowledge.
>spent the last year fixing stuff educating people on principles and qualities.
> countless hours of training and explaining. team is showing cooperation, a new requirement comes in to develop with react.
> tear my ass creating reusable shit and self explanatory code with proper naming etc using git with feature branching, monday is first deployment day.
> today a colleague was working on an item submit a pull request and self approve it
> look at the code..... WTF the dumb fuck copied and pasted the whole code from different kendo components but somehow managed to refractor the name to test component, commented out all the code that he didn't use did the api call directly from the component, has 2 useeffects that depends on the a fucking text box changes for no reason, no redux implementation, the acceptance criteria is not achieved, and it doesn't work it just look right.
> first world country shit cannot scold, cannot complain, lead by example.
>asked him why you did this, the response was yeah probably i shouldn't have done that, i really didn't understand anything in the training but didn't want to waste time!!!!
> rest of the team created a different styled disaster with different flavors they don't even name their shit the same way.
fellow developers I'm stuck in a spaceship with a bunch of imposters, seriously i never cried in my entire life now I'm teary and on the verge of a break down.
talk with management "improving needs time" and offers me to join a yoga session to release the stress as if reaching nirvana would deliver shit on monday.
i really don't know what do is this a rant, is this a cry for help, I'm not sure, any advice is welcomed.7 -
Recently I've started thinking about how we are always told "No you can't do that" to everything. That feels like a theme in our industry.
I've also been thinking about how often people say well done to each other, or just comment that something is good in a pull request. Everything is always focused on bugs and mistakes - not good bits.
The first point conflicts with the idea that when using languages and frameworks you should follow their philosophy or you're gonna have a bad time - but in all other instances you mostly don't have wrong answers, just answers that can be better so a lot of stuff is opinion based.
I've decided to change my ways and focus just as much on good stuff as bad when I review code and to make sure I'm focusing just as much, if not more, when people do something good.
I think I do a good job, but I don't think I've been told I'm doing a good job or that anything specific is good more than a couple times in the last year - mostly in mandatory reviews. What about you?2 -
A bug is born
... and it's sneaky and slimy. Mr. Senior-been-doing-it-for-ears commits some half-assed shitty code, blames failed tests on availability of CI licenses. I decided to check what's causing this shit nevertheless, turns out he forgot to flag parts of the code consistently using his new compiler defines, and some parts would get compiled while others needed wouldn't .. Not a big deal, we all make mistakes, but he rushes to Teams chat directing a message to me (after some earlier non-sensible argument about merits of cherry picking vs re-base):
Now all tests pass, except ones that need CI license. The PR is done, you can use your preferred way to take my changes.
So after I spot those missing checks causing the tests to fail, as well as another bug in yet another test case, and yet another disastrous memory related bug, which weren't detected by the tests of course .. I ponder my options .. especially based on our history .. if I say anything he will get offended, or at best the PR will get delayed while he is in denial arguing back even longer and dependent tasks will get delayed and the rest of the team will be forced to watch this show in agony, he also just created a bottleneck putting so many things at stake in one PR ..
I am in a pickle here .. should I just put review comments and risk opening a can of worms, or should I just mention the very obvious bugs, or even should I do nothing .. I end up reaching for the PM and explained the situation. In complete denial, he still believes it's a license problem and goes on ranting about how another project suffering the same fate .. bla bla bla chipset ... bla bla bla project .. bla bla bla back in whatever team .. then only when I started telling him:
These issues are even spotted by "Bob" earlier, since for some reason you just dismissed whatever I just said ..
("Bob" is another more sane senior developer in the team, and speaks the same language as the PM)
Only now I get his attention! He then starts going through the issues with me (for some reason he thinks he is technical enough to get them) .. He now to some extent believes the first few obvious bugs .. now the more disastrous bug he is having really hard time wrapping his head around it .. Then the desperate I became, I suggest let's just get this PR merged for the sake of the other tasks after may be fixing the obvious issues and meanwhile we create another task to fix the bug later .. here he chips in:
You know what, that memory bug seems like a corner case, if it won't cause issues down the road after merging let's see if we need even to open an internal fix or defect for it later. Only customers can report bugs.
I am in awe how low the bar can get, I try again and suggest let's at least leave a comment for the next poor soul running into that bug so they won't be banging their heads in the wall 2hrs straight trying to figure out why store X isn't there unless you call something last or never call it or shit like that (the sneaky slimy nature of that memory bug) .. He even dismissed that and rather went on saying (almost literally again): It is just that Mr. Senior had to rush things and communication can be problematic sometimes .. (bla bla bla) back in "Sunken Ship Co." days, we had a team from open source community .. then he makes a very weird statement:
Stuff like what Richard Stallman writes in Linux kernel code reviews can offend people ..
Feeling too grossed and having weird taste in my mouth I only get in a bad hangover day, all sorts of swear words and profanity running in my head like a wild hungry squirrel on hot asphalt chasing a leaky chestnut transport ... I tell him whatever floats your boat but I just feel really sorry for whoever might have to deal with this bug in the future ..
I just witnessed the team giving birth to a sneaky slimy bug .. heard it screaming and saw it kicking .. and I might live enough to see it a grown up having a feast with other bug buddies in this stinky swamp of Uruk-hai piss and Orcs feces.1 -
So my friend who is currently attending University to major in Computer Science just started programming Java a few days ago. His first assignment was to learn bubble sort and make it organize a table of certain values provided in the assignment with a few other items on the side. Apparently, he was stressing over the assignment and waited till the last night to do this, and was running on 2 hours of sleep. Anyways, a few days pass and he received a 0% on the assignment with the comment "See me on Monday." and questioned what he did wrong (They use GitHub to submit their assignments, even though other classes at the University just commit to the University Server for Computer Science), and asked me to review the code. When I started looking at the code, all he managed to do was just make two tables, one that would print the unsorted table, and then print the "sorted" table. Plus, the catch that got him in trouble, he named his package "fuckthisshit", how does one not realize that when they're submitting their assignments... like seriously? Like I can understand the 2 hours of sleep, but with 1000s of examples out there, how do you manage to fake bubble sort plus end up naming a package "fuckthisshit" and question why he got a 0%. I do feel bad for him in the long run since there aren't many assignments in this class so this was worth 25%.
-
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 -
So I did a code review for a colleagues pull request and I've noticed that he hasn't written the PHPDocs for a lot of the classes and functions. One minor thing I wrote is to add the author for the class.
About 2 mins after writing that comment he came over to tell me why should he write the author in the comments when people can just go look at the git commit logs. I was like WTF? I asked why would he do that, his answer was that if there's an issue, we can just use git blame to identify the author. To me that makes no sense as git blame isn't supposed to be used like that.
It's guys like these are the ones who don't document anything whether in an online document or even in code. And they just make work harder for the rest of us.2 -
Comment on a code review:
How does this relate to the task?
Me:
Most of the changes have nothing to do with the task, but half of the code & build system was either wrong, broken or not following best practices. This particular change is because something was broken.3 -
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 -
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...
-
Me: [jira comment] We have similar text for the mobile version of the site already. [includes screenshot of what site looks like now] Are you sure about this?
[radio silence for a few hours]
Me: [slack] I want to follow up.
Web Operations: What’s the issue?
Ooh k. Slack messages can have a tone.
Me: I just want to confirm we’re not repeating copy.
Web Ops: We’re not.
I complete the ticket and submit for review. The C-suite for my department reviews.
C-suite: [to web ops in JIRA comment] This looks weird. Is this right? [sends screenshot of my work because there is repeated copy, like I said there’d be]
Web Ops: [in JIRA comment] Oh, I thought X was questioning the request. X changed the wrong text.
C-suite: The website has always looked like that. You’re looking at X’s screenshot for the current website. Look at the screenshot I sent over.
Later, I complain because web ops was completely unprofessional with the comment about “questioning the request.”
C-suite: Web Ops is working hard. It’s our busy season and it’s their first time dealing with it. You know, I’m going to teach them some css and html so they can make content changes in the CMS and they’re not sending over changes so often and bothering you.
Me: [to myself] 🤨 wtf so it’s ok for web ops to treat me like dirt. And in writing. And with service that’s version controlled—JIRA emailed web ops comment to me. And lol no 😂 on teaching them how to code. That’s such bullshit. We all know you’d never allow them to edit the CMS because they’d fuck up the site. And they wouldn’t do edits anyway because it’s beneath them. And idk how this relates to web ops gross behavior.
A few days later.
Me: I was offered a job elsewhere. Here’s my two weeks notice.
C-suite: Can you push back your last day? It’s our busy season.
Me: Nope. Bye Felicia.1 -
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 -
Good code is a lie imho.
When you see a project as code, there are 3 variables in most cases:
- time
- people / human resources
- rules
Every variable plays a certain role in how the code (project) evolves.
Time - two different forms: when certain parts of code are either changed in a high frequency or a very low frequency, it's a bad omen.
Too high - somehow this area seems to be relentless. Be it features, regressions or bugs - it takes usually in larger code bases 3 - 4 weeks till all code pathes were triggered.
Too low - it can be a good sign. But it should be on the radar imho. Code that never changes should be reviewed at an - depending on size of codebase - max. yearly audit. Git / VCS is very helpful here.
Why? Mostly because the chances are very high that the code was once written for a completely different requirement set. Hence the audit - check if this code still is doing the right job or if you have a ticking time bomb that needs to be defused.
People
If a project has only person working on it, it most certainly isn't verified by another person. Meaning that only one person worked on it - I'd say it's pretty bad to bad, as no discussion / review / verification was done. The author did the best he / she could do, but maybe another person would have had an better idea?
Too many people working on one thing is only bad when there are no rules ;)
Rules. There are two different kind of rules.
Styling / Organisation / Dokumentation - everything that has not much to do with coding itself. These should be enforced at a certain point, otherwise the code will become a hot glued mess noone wants to work on.
Coding itself. This is a very critical thing.
Do: Forbid things that are known to be problematic in the programming language itself. Eg. usage of variables in variables, reflection, deprecated features.
Do: Define a feature set for each language. Feature set not meaning every feature you want to use! Rather a fixed minimum version every developer must use and - in case of library / module / plugin support - which additional extras are supported.
Every extra costs. Most developers don't want to realize this... And a code base that evolves over time should have minimal dependencies. Every new version of an extra can have bugs, breakages, incompabilties and so on.
Don't: don't specify a way of coding. Most coding guidelines are horrific copy pastures from some books some smart people wrote who have no fucking clue what you're doing and why.
If you don't know how to operate on people, standing in an OR and doing what a book told you to do would end in dead person pretty sure. Same for code.
Learn from mistakes and experience, respect knowledge from other persons, but always reflect on wether this makes sense at this specific area of code.
There are very few things which are applicable to a large codebase on a global level. Even DRY / SOLID and what ever you can come up with can be at a certain point completely wrong.
Good code is a lie - because it can only exist at a certain point of time.
A codebase should be a living thing - when certain parts rot, other parts will be affected too.
The reason for the length of the comment was to give some hints on what my principles are that code stays in an "okayish" state, but good is a very rare state -
One day, the Director of Web Ops (marketing role) submitted a ticket to update the list of product categories on the website’s navigation. Sounds like a simple ticket right? Just some html edits. Nope. Every day for three days, she changes her mind and adds new changes. What should have taken me 10 minutes stretched out to three days. She held up code review of my ticket because she kept making changes.
She had plenty of time to sort out what she wanted. That ticket had been sitting in the To Do pile for two days before I touched it.
She was being an asshole because she knew she could get away with it and I had no recourse: my direct manager was on vacation, the entire dev team was going to be laid off anyway so no one was going to defend us on “trivial” matters, and we were going to enter code freeze soon so she’d just argue it was critical business changes for our critical revenue season.
I suspect she was also just not good at her job. I never met her in person because she was hired during the 2020 pandemic and we were all working remotely. I did see her make a five minute presentation during an all staff meeting…and she didn’t come off too well. Her voice was trembling during her turn to speak…like she was not confident or not prepared.
She knew she was causing chaos but she put on this act of not knowing. She was definitely trained on our dev team’s practices for tickets and deployments. She knows about code review, beta testing, and user acceptance testing that has to happen before a ticket can be deployed.
It happened to be before Thanksgiving weekend 2020. Our deploy was going to happen on Tuesday instead of Thursday because Thursday was a holiday (no one would be working) and Wednesday was a half day.
Tuesday afternoon at 1pm, she messages me and the dev in charge of deploy about more changes! My time is already occupied because our Product Manager went on vacation and dumped a large amount of user acceptance testing on me. I scream at my computer at that point because I realize I’m in the ninth circle of hell. I tell the other dev in a separate message that Web Ops has been making changes EVERY DAY since I picked up that ticket.
Other dev tells her that we have to check with the C-suite executive for engineering because we’re not allowed to make changes to tickets so close to the deploy. This is actually the policy. He also tries to give Web Ops the benefit of the doubt because we’re not deploying on our usual day. He had to do that to so she didn’t feel bad (and so she doesn’t complain about us not working towards the company’s goals).
Other dev had to do the code changes because I was otherwise occupied with user acceptance testing. If I were him, I’d be pissed that I was distracted from concentrating on the deploy so close to the holiday.
Director of Web Ops was actually capable of even more chaos. I ranted about it before. For that dramatization and if you want to go down the rabbit hole, see: https://devrant.com/rants/4811518/...4 -
Update in earlier rant about work.
So I've basically made an flux store using Rxjs. Using symbol's as tagret data and an action. I asked my boss and another work colleague to do code review on the new system as I'm not sure their like it or understand what i was doing.
Passed fined.
2 weeks later i get a comment in some code that implement in basically asking whats all this and what is it doing.
-_- it's called really tidy shorthand code aha -
1) receive functional requirements
2) create functional specification, post it on forum (no jira)
3) create memo document, post it on forum (no jira)
4) create analysis document with actual code changes without seeing the code (wait for step 8), post it on forum (no jira)
5) receive review on analysis document, fix it and post (no jira, redmine etc from now till the end of rant)
6) after analysis is approved make a checkout request
7) source code manager checkouts files from svn and posts them on forum along with the files list
8) you make actual changes to the code, post changed sources on forum
9) source code manager makes a review to check that amendment commet is present in source code and is properly tagged, and every line of code chnged is properly tagged (you are not allowed to delete anything, not even one space, you need to comment it (and put an appropriate tag))
10) after you passed review you fill in standard compilation request form
11) you code is compiled and elf is put on testing stand
12) you fill in "actual behaviour" and "expected behaviour" columns near description of changed function in template of unit test plan document (yeah we have unit testing) and post it on forum
13) if testing ok changed sources and compiled elfs along with its versions (cksum) commited to svn (not by you, there is a source code manager for that)
14) if someone developed function in same source file as you "commited" he is warned by source code manager and fills checkout request form again
15) ...2 -
grrrr
last week my laptop died out of nowhere. it stopped recognizing the one drive in it. I lost a bunch of files, code. evidently ssds fail out of nowhere unlike hdds which slow down and error all the time before ultimate failure
my warranty for this 4k$ laptop expires in 12 months and this was month 13. nice. I don't like warranties anyway, and the site said they would replace things with "comparable hardware, sometimes refurbished" wtf no thanks
so I found some guides of people upgrading the drive in this laptop. seemed easy enough, unlike older laptops from back when I was in school where you had to take out 12 things first to get to anything
unfortunately I needed a specific screwdriver. I walked several miles to the nearby hardware store thinking they would have said screwdriver. the old guy in the basement said there was a kit where it started from t4 (I needed t5), but he had just sold out his last one. I checked their online store with a friend for a while on my way back home and we kept finding torx screws but the wrong sizes. fuck.
he said screwdrivers this small are only used for electronics, asked if there's any other hardware stores and there aren't near me
however it occurred to me this strip mall has a lot of suspicious computer stores on it. so I walked back up the street looking for one.
found one with a suspicious poster, saying it was an internet cafe but the last point on their poster said they do repairs. walked in. nobody is in there, suspiciously 2 desks with old computers all empty, then you go forward in this dark cave, with plastic wrapped implements on the walls, you finally find a glass shield and behind it was a meek Asian man that took me a moment to notice
I asked him if he had t5
he handed me a plastic baggy full of tiny screwdrivers, for me to take one
I asked if they're t5
the shape looked right, but I can't tell the size
I took one out and tried to find size marking, but nothing
he didn't seem to know what I was asking when I asked about its size
he said if it's wrong I can come back and trade what I took for another. lol
I asked him if I can buy it, since that wasn't evident to me due to how sus this random bag of screws is being thwarted on me lmao
he said 5$ cash
I gave him a fiver
this sus shop literally avoiding taxes lmao
walked back home, ate food cuz starving, tried the screw and FUCK, it's too big. put laptop in a bag and hauled ass fast, checked on maps the store I got this from closes in a few minutes so I really wanted to make it there because what if the receptionist changes and they don't know I took this screw. I got no receipt
got there right before closing, put my laptop down, said it was too big. he used a few screws until he found one that fit, said I could try it and I did (so scam aware!). bingo bango. now I got a screwdriver that fits the laptop.
walked home, sat down and took apart the laptop. been a few years since I did so. the hardware inside looks entirely unrecognizable to me. started cycling through YouTube videos of laptops of the same name as mine, but their insides don't look like mine. is this ram? is this the NVMe? what the fuck is anything?
finally found a video guide where the guy was quite informative. not the same laptop but he's informative enough I figure it out. ram and drives are so different and weird now. took parts out, put them back in, rebuilt laptop, tried to boot, same problem. jiggling parts like this works with desktops often, guess not with a failed NVMe
so I'm screwed. get on Newegg and bought a new NVMe. should arrive in 3 days via Purolator
yesterday was day 3. it was at a sort facility near me, then out on delivery, but nobody ever came. then it went back to sorting. now it's out on delivery again. I'm sitting here thinking that's a little weird, wasn't Purolator the delivery company that had me go 2 hours outside of town to pick up a 15lb desktop case once?
... and then I looked up Reddit comments... then reviews on the purolator facility it's at... I am screwed. last time iirc they were out for delivery for 3 days, never tried delivery, then on the last day at the end of day they stated they attempted delivery but no go. that was bullshit. then it ended up at that facility. which takes 2 hours to fucking reach.
the reviews are so bad... the facility has 1.2 star reviews with thousands of them. they won't leave even a stub, then seem to not know where your package is at the facility, or they deny you have the right to pick it up despite ample IDs, or someone ELSE picks it up and it's not there. they also ship your package back after 5 days, so if they don't leave a note and you miss it tough luck...
fucking hell
also rumours that they just hire "contractors" in normal cars to drop off packages? wat? lol
AND EVERY REVIEW HAS A BOT COMMENT. THEIR SUPPORT IS JUST A CHATBOT
I thought this was just a small hiccup
I think I might not have a drive for weeks now
fucking hell
now I'm sitting on my porch2 -
Things that annoy me about my current place:
1 - Only 3 people out of a team of 12 developers are allowed to purge akamai, or merge pull requests to master on any of our 200+ websites. Apparently this is because us contractors are not allowed because the permanent employees have to be accountable for the code.
Despite this, no-one actually reads the code. You just throw up a request in the slack channel and boom, instantly 30 seconds later someone approves it, even if its 500+ lines of code.
2 - I've pushed for us to move to agile instead of waterfall, and got declined (which is fine), but the reasoning was that the dev team are not 'mature enough' to work that way. Half the devs here have 5+ years of experience so I don't get the problem here.
3 - There is zero code reviewing process in place. I just watched as a developer's 300 line merge request was approved within 8 seconds of it going live. No-one is allowed to comment on the code review or suggest changes as this would 'slow down development'. Within that 300 line merge request were tons of css being aimlessly commented out, and invalid javascript (introducing both bugs and security issues) that were totally ignored.
What is your thoughts on these above points?
Am I too narrow minded or is the development manager clueless?1 -
i am feeling angry and frustrated. not sure if it's a person ,or codebase or this bloody job. i have been into the company for 8 months and i feel like someone taking a lot of load while not getting enough team support to do it or any appreciation if i do it right.
i am not a senior by designation, but i do think my manager and my seniors have got their work easy when they see my work . like for eg, if on first release, they told me that i have to update unit tests and documentation, then on every subsequent release i did them by default and mentioning that with a small tick .
but they sure as hell don't make my work easy for me. their codebase is shitty and they don't give me KT, rather expect me to read everything on my own, understand on my own and then do everything on my own, then raise a pr , then merge that pr (once reviewed) , then create a release, then update the docs and finally publish the release and send the notification to the team
well fine, as a beginner dev, i think that's a good exercise, but if not in the coding step, their intervention would be needed in other steps like reviewing merging and releasing. but for those steps they again cause unnecessary delay. my senior is so shitty guy, he will just reply to any of my message after 2-3 hours
and his pr review process is also frustrating. he will keep me on call while reviewing each and every file of my pr and then suggest changes. that's good i guess, but why tf do you need to suggest something every fucking time? if i am doing such a shitty coding that you want me to redo some approach that i thought was correct , why don't you intervene beforehand? when i was messaging you for advice and when you ignored me for 3 hours? another eg : check my comment on root's rant https://devrant.com/rants/5845126/ (am talking about my tl there but he's also similar)
the tasks they give are also very frustrating. i am an android dev by profession, my previous company was a b2c edtech app that used kotlin, java11, a proper hierarchy and other latest Android advancements.
this company's main Android product is a java sdk that other android apps uses. the java code is verbose , repetitive and with a messed up architecture. for one api, the client is able to attach a listener to some service that is 4 layers down the hierarchy , while got other api, the client provides a listener which is kept as a weak reference while internal listeners come back with the values and update this weak reference . neither my team lead nor my seniors have been able to answer about logic for seperation among various files/classes/internal classes and unnecessary division of code makes me puke.
so by now you might have an idea of my situation: ugly codebase, unavailable/ignorant codeowners (my sr and TL) and tight deadlines.
but i haven't told you about the tasks, coz they get even more shittier
- in addition to adding features/ maintaining this horrible codebase , i would sometimes get task to fix queries by client . note that we have tons of customer representatives that would easily get those stupid queries resolced if they did their job correctly
- we also have hybrid and 3rd party sdks like react, flutter etc in total 7 hybrid sdks which uses this Android library as a dependency and have a wrapper written on its public facing apis in an equally horrible code style. that i have to maintain. i did not got much time/kt to learn these techs, but once my sr. half heartedly explained the code and now every thing about those awful sdls is my responsibility. thank god they don't give me the ios and web SDK too
- the worst is the shitty user side docs. I don't know what shit is going there, but we got like 4 people in the docs team and they are supposed to maintain the documentation of sdk, client side. however they have rasied 20 tickets about 20 pages for me to add more stuff there. like what are you guys supposed to do? we create the changelog, release notes , comments in pr , comments in codebase , test cases, test scenarios, fucking working sample apps and their code bases... then why tf are we supposed to do the documentation on an html based website too?? can't you just have a basic knowledge of running the sample, reading the docs and understand what is going around? do i need to be a master of english too in addition to being a frustrated coder?
just.... fml -
Oh boy this may be my best product review yet. I'm totally smitten with GitHub Copilot! I always put off trying it, but I finally gave it a try recently. Man, oh man, once I got a taste of it, there was no going back. This auto-suggest feature is pure sorcery! It throws out complete function suggestions while you type, and it's all based on the context of your code.
Let me tell you if you have never tried it, it's freaking awesome and super handy! I've been learning Python for less than a month, but thanks to the freaking Copilot, my Python skills have skyrocketed like for real. I know this because I tackled a Python project and nailed it. The client was stoked because it worked flawlessly, even though my Python skills are still a bit rough around the edges.
The coolst thing is hw clean my code looks, especially for a beginner. all I have to do to add a comment is type a double slash, and Copilot takes care of the rest. It suggests what should go on each line as I type, and it's scarily accurate.
You know what's wild? On the GitHub page, it claims that Copilot writes 50% of the code. But, dude, for me, it wrote way more than that!8 -
I am new here so apologies if I make any mistakes.
I have been a opensource contributor since last 2 years and it has been a great experience. As I am looking for a new opensource organization, I got around an organisation X(name changed). It is my first time when I don't like an opensource organization. The organization is controlled bh a single person and he does just tells me to copy the whole website of another popular opensource organization and make the organization website. Also, he does not listen about anything. He just pings me about the work done everyday even after telling him that a review is a blocker for me to do new task. I don't say it is a bay thing but don't looking at the issue is the main thing. On another case, the build pipeline was failing. It can be solved only by changing certain settings on the build pipeline and I does not have its access. I told him about how to tackle it in the review comment. Even after this, he just pings me for around a week just telling me that it has something to do with my code and the pipeline is all right.
I can understand that in the early phase, an organization may have some problems and the setup may have some flaws but this type of dictator behaviour is not good in my opinion. I had worked in 3-4 opensource organization and all have very welcoming community. I had always learned from them but this is my first time bad experience with it3 -
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