Reviewers on the internet in a nutshell

Trying to keep my code reviewers entertained:

Don't be lazy when naming things. You yourself will be lost in all the a, b, c, ds, let alone your code reviewers.

I jump on an existing scala project.

git pull && sbt compile test

Tests are failing.

Me: "Hey team, the tests are failing."

Team member: "That cannot be. They were passing for the the last run."

Me: "Did you run them locally?"

Team member: "No, on Jenkins. It was fine."

I check Jenkins.

Me: "What do you mean it's fine. The last successful deployment was on the end of May."

Team member: "The Pull Request checker always went through successfully."

I check how our Jenkins tasks are configured. It's true that the Pull Request Checker runs successfully yet due to a "minor misconfiguration" (aka "major fuckup") the Pull Request Checker only tests a tiny subset of the entire test suite.

Team members were were fine if their Pull Request got the "Success" notification on bitbucket's pull request page. And reviewers trusted that icon as well.

They never checked the master run of the Jenkins task. Where the tests were also failing for over a month.

I'm also highely confused how they did TDD. You know, writing a test first, making it green. (I hope they were just one specific test at a time assuming the others were green. The cynic in me assumes they outsourced running the tests to the Jenkins.)

Gnarf!

Team member having run the tests locally finally realizes: "The tests are broken. Gonna fix them."

Wow. Please, dear fellow developers: It does not kill you to run the entire test suite locally. Just do it. Treat the external test runners as a safety net. Yet always run the test suite locally first.

Pro tip for job candidates:
If you push a code challenge to a live hosting service like github pages or S3, don’t give the reviewers a link to the repo!! Instead put the link into the home page and send the reviewer only a link to the live hosted page.

Why?

Because, if you host with github pages, you’re required to use the project path as the domain root. If the reviewer pulls your project and doesn’t bother to read your readme file with the link at the top, he’ll complain that he couldn’t figure out why your project isn’t hosted from the root domain, and he’ll pass on your application.

True story.

Biggest challenge was to accept that no matter where you go, what you do, you will have to deal with stupid people.

They come in all ranges, starting from hr, interns, juniors, coworkers, code reviewers, seniors, managers, boss, clients.

And in order to grow in life, you have to learn to deal with them. Better even if you could make them a little less stupid.

Had a five hour long debate with one of our Senior Developers today about pull request etiquette.

His view was reviewers should always email or call him before adding comments to any of his requests and they should never block them as he should be allowed to code in "his own style" and should be able to approve his own pull requests.

I explained that we have code standards and an agreed PR workflow be needs to comply with.

He then started talking about meteors and plane crashes. Literally no helping some people.

Are most Macbook reviewers retards

this kind of pull request reviewers.. lol

Watching reviewers on a website review the new Eminem album 12 hours after its release is like watching a 10 year old review a scientific paper by Stephen Hawking with a quick skim.

Yes, the child should be entitled to their opinion. But No, no one with any sense should be listening to this child’s opinion.

If you gave a lil pump “album” a 7/10 and Kamikaze a 4/10 then your opinion is invalid, end of story.

But it’s all good. 95% of people are stupid and that’s just how it works, it’s nothing to be angry about. Just try to be in the 5%.

End of rant.

When it comes to nervewrecking situations, what I've encountered so far is a pull request for which the 2 reviewers had opposite opinions.
Something like this:
* opens pull request *
reviewer 1: "update using approach A"
* updates code *
reviewer 2: "this is wrong, change to approach B"
* updates code again *
reviewer 1: "this isn't what i asked for, i'm rejecting your PR"
Oh, also, each of them had their own set of coding standards.

What was your most challenging situation when writing code?

Hmm. Seniors have the half working experience as I do and I am the only not senior one in Dev team.

I adapt code only to "taste" good for code reviewers, but they allow themselves to commit without caring and just saying :"oh unit testing is boring"

Enough with the kindergarten. Time to prepare myself for the next job.

My paper just got rejected. Again. The first time it was expected. But for this journal, it wasn't supposed to be. Some of the reviewers' comments are stupid. (for eg. I mention a no-loss algorithm fir a game which, so his/her comment is like what's no-loss? , like are you fucking kidding me, if you don't know that, then why are you a reviewer in the first place)

Anyway now I don't know what to do. I'm looking for more journals but all have so high impact factors and I'm not even sure confident to submit again. Had a good mind to mail the editor in chief but well, I don't think it'd help. What do you guys think?

In the middle of another project, another paper, online courses, now this. I'm just done. I didn't go home as well. It's around four o' clock in the morning here, so noone here is awake.

Can anyone hear me?

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 review

Why is it that the tech Youtubers of this world (and tech reviewers in general) tend to completely skip development as a use case, and instead (if they do ever move off gaming) focus on things like Rendering & Modelling / CAD work? I'm sure there's *way* more devs in the world than CAD guys, surely?!

And if they *do* give it the light of day, it's always a quick benchmark based on "Firefox compile time", "Linux kernel compile time" or similar. Dude, it's 2020. Much as some would like to believe otherwise, most guys stopped compiling swathes of heavy C & C++ as part of their normal workflow over a decade ago.

Real-world tests I want to know about are things like docker performance, common IDE startup performance, compile performance of different sized applications on a bunch of langs like Kotlin, C#, Java, Clojure - or node.js performance, Tensorflow performance on NVidia's vs AMDs latest GPUs, etc. I care about how many IntelliJ instances & VMs I can have open way more than how many Chrome tabs I can forget to close.

But noooo - forget that, here's how fast Blender can render a BMW! 😬

The worst of Agile and Sc(r)um: All those people knowing the right way(™) to do it. Endless discussion about useless tooling: the proper use of the custom workflow in Jira, on when and how to create sub tickets. The hour-less meta-discussions on what should be discussed where and when (what's subject of the backlog refinement, retro, etc), the roles: the PO's, what he should do, cannot, the PM's. Who is allowed to pull a ticket to the sprint or not. How many reviewers need to acknowledge a pull request. To and fro. Pointless, but fought with heart and blood, full of sound and fury, signifying nothing.

And everywhere I hear: "In my previous company, we did Scrum like.. and it worked perfectly!"

Some of you might remember my rants on Mr. Gitmaster, with whom I thought I'd made my peace. Guess what? He's now a team member and turning into Mr. Agile - a more severe reincarnation! As our company starts flogging that dead horse of Agility, he seems to feel strong tailwind. Our team lead would constantly cut his monologues, but he's now on holiday, so we have no escape from the never ending: "In my previous company..."

If it was so great, why didn't you stay?

We are not allowed to pull a ticket to the sprint unless every team member is notified? I don't fucking care. If our software fails on customer's machines and I can fix it, I will do if there is a ticket, if it's in the sprint or not. Screw Scrum, if it is getting in the way of it. You can waste your hours discussing horseshit, I want to sit at my desk, deep in the test-compile loop and ship some fucking code.

In my PR :

Senior dev A : "You should change the format according to <link to coding standard>"
Me : "But it doesn't mention anywhere about that format. <senior dev who wrote the standard> also agrees with me. Other reviewers also already approved."
Senior dev A : **proceed to give me an example from a file that's not even in the PR scope**
Me : "I cannot find that file in my PR"
Senior dev A : **give me another example example from my PR**
Me : "Okay I missed that, I am gonna fix it, but other files are already using consistent format. I have already merged changes for 500 files using this format, and I still have 400 files to go.
Do you really want me to revert the changes from 500 files?" :/
Senior dev A : "I don't want to be your enemy, I just want to make our codebase better"
Me : **Mad because he took this personally.**
**I don't want to be your enemy either. I also care about the codebase. I just want to finish this ticket ASAP instead of implementing your cosmetic changes that's not even in the standard so that I can work on another ticket that will have more impact to the company**
Senior dev A : "Ok, I will approve it, just add some whitespaces"
Me : 🤦‍♀️

I sometimes think that some senior dev just want to flex when they're reviewing PR.
They just want to let people know they wield the power.

I got assigned to work on a new project a couple of weeks ago. We got the POC code handed off from senior management, since he came up with the idea over the weekend. The project concept is hella exciting, but the dev manager and PO I have to deal with make life unbearable to say the least.

We have only 2 devs (including me) and 1 QA on this supposedly very important project. Of course, management announced the project to the clients already, so now we have to deliver ASAP cause it adds “sizzle”.

The MVP deadline is... no one knows when, either July 30th or September 1st. The MVP requirements are... unknown. I swear if someone saw the list of tasks and issues attached to “MVP” Epic, they would call us nuts trying to fit it all in.

To make things better, each PR requires 2 reviewers, so we end up adding manager as a reviewer just cause we need him to hit that “approve” button. So in attempt to make life easier, we requested to have a third developer. We are getting another developer, but that guy doesn’t know how to unit test a pure function...

Current priorities are... unit testing with coverage of 95% and if we want to refactor code, we have to add area to the list in a Google Doc. As a result, we are not tackling big things like risk of SQL injections not to mention big features like i18n (5-6 languages to support by the way and yes, it’s part of MVP as well as SSR no one knows why). Currently, I spend 2-3 hours a week in calls with the team just to figure out what the hell MVP is, what we have to do and why we have to do it. Last time we spent an hour refining 1 spike and breaking down one story into 3.

Oh, we also don’t have a deployment plan, not even to test environments since DevOps team was not aware of this project at all. Thus, QA cannot create any test suites and have to test everything manually which eats a lot of their time.

This whole project is a big hot mess and I’m considering leaving it all together especially since I’m working on two squads at the same time. I love the project, I love the idea, but management makes it unbearable, so I’m not even motivated to work on that.

I was working under the title of mid-level developer when I felt I was clearly still a junior and was tasked with making changes to the core search functionality of the company website. After it was 'reviewed' by 3 people more senior than I and merged to production it took the majority of the site down.

I got the blame and the reviewers weren't even questioned 😒

How to get a fast MR approval 101:
Tell the reviewers that you've had a trivial merge conflict due to other colleague adding unit tests in the same file as yourself, which causes the approvals in Gitlab to vanish.
They assume they had given the approval previously and just push the button.

Dear reviewers, feedback givers, coworkers, clients and critics...

There is a subtle difference between constructive criticism and just spewing hate!

One offers advice & points in the direction of a better solution, while the other one makes me value you less, as a human being...

If your comment/review/feedback is:
“I don’t like it, I don’t know why, I just don’t” - please keep it to yourself because it makes us less likely to listen to you when you actually have something worthwhile to say...

Yours,
Any and all creative professionals,
including developers.

Was recruited to build a text-based course where I get a nice bonus if I finish the course early. Now I know how they are always able to save themselves from giving that out. There's so much fucking red tape for each literal sentence I write! I have MULTIPLE reviewers, commenting, editing, and "suggesting" EVERYTHING I write.

News flash: this course is derived from a different video-based course that has sold hundreds of copies on other platforms, so I must be doing something right.

Just let me write the whole course and we edit it in the end!!! This treadmill is going to triple or quadruple the time until publishing...

I feel like I'm trapped in the movie office space: "every day I have 5 different bosses come and tell me the same thing"

Won't be working with this platform again.

F the app store. My app is getting rejected because they claim 10% of my product should not need an user account to view. Now I have to spend another week changing my frontend and backend, delay my launch, just to satisfy what is essentially a stupid Apple-only UX requirement. This exact same binary was approved a week before... Seriously, screw the reviewers and the high horse executives at Apple.

Car reviewers are the bane of my existence.
In one video, a dude reviews a 3rd gen Rav4 and says "don't expect Porsche like performance or luxury materials".
Well, no fucking shit, Sherlock.
Who would've thought? An affordable compact SUV doesn't have sports car performance. Mind = Blown.

Also, vague and subjective criticism such as "unimpressive steering" or "not fun to drive". Not fun how? Whatever the fuck that means.
Dudes want a budget car with a Bugatti engine, that handles like a Porsche but priced as a Honda.

Seriously, why the fuck do these reviewers regurgitate the same shit, over and over again.

A good review must take into account the price of the car. And at most, compare it to similarly priced cars.
Comparing a Corolla to a 911 is down right moronic.

System Engineer who is adhoc scrummaster got all pissy when us devs did not transition their jira tickets when they merged with develop.

Jesus christ take five minutes, google it and figure out how to do it automatically and while you are add it add the fucking reviewers!

It’s a pain to do it each time!!!!

Fuck!!!!!!!!!!’

Glassdoor gave Zuru the names of the bad reviewers, what a (non-)surprise!

Not glassdoor’s “fault” as it was forced to, but… hey, it was to be expected when you bind a review to an account 🤫

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.

When the code reviewers ask for waYyYyY too much testing information and you end up doing all the QE heavy-lifting.....

Two reviewers two comments on the same content, both comments conflict with each other and I'm required to apply both, how exactly?

Today's mood is "I'm just gonna try my best and hope my reviewers don't call me out on my shit"

I hate this crap and wish people would stop doing it. It makes my brain bleed and doesn't prevent any difficult to find bugs.

if (TERMINAL_COUNT <= index_thing)

English doesn't work that way, and I don't know about you but this crap is just awkward as hell. Sweet Jesus I wish there was less cargo cult programming in the world. Just because you saw something in a blog that convinced you that reverse comparisons is best doesn't mean it actually is. Use a damn static analysis tool to catch accidental assignments in expressions, don't twist my brain to interpret your weird phrasing of comparison operators. Some of your code reviewers may be dyslexic and have enough problems as it is.

And now for the mini-rant that I'm actually here for: You know what makes for difficult to find bugs? (Hint: It sure as hell isn't an assignment in an expression) Releasing an RTEMS semaphore you've never obtained. You'd think that would cause some kind of panic or assert failure but nope. Instead it causes... misaligned address exceptions? In statically allocated global memory? WTF??

I’m the only junior software engineer at a small startup where I do mostly web development, as well as other bits and pieces (automation, ci/cd, etc)

Our software team is extremely small so we do not have anyone dedicated to QA. I usually just ask a team members with related experience to review my merge requests. So if I have a merge request for our ci/cd, I ask the software engineer with the most ci/cd experience to review the MR.

Recently I realized that my MRs will usually sit for days, and sometimes weeks without the reviewers taking a look. And when they eventually do, they don’t even run the code. It seems like they just gloss over it and look for obvious syntax or logic errors.

It makes me feel as if my code and efforts do not have much value to our team.

It also pisses me off because whenever a issue happens in our codebase, me and my code is the first thing blamed even if my code is not the issue

Is this typical in other companies? Or is this something I should speak to my boss about?

Ugh, no.. You got yourself from dirty dental-dams of “Jif” believers, all the way to passed bass-ackwards. That PR is raw sewage and your demoted back to entry-level if not intern. Oh and while you’re crying with Morty’s dad; consider a career change. Most importantly, just forget WAN ever existed and go back to your parents first PAN device to play solitaire.

-

Sincerely,
All engineers, reviewers and testers who clean up your leaking dirty diarrhea lines.

I didn't get into GSoC while writing code which was to be a major aspect of the next release of SymPy. I tell you this org. is maintained by 1 maintainer and 4-5 other members. While most don't understand the code written but will teach you to write some decorator class. I don't want to name that sucker,but he made some changes and then other reviewed and told to change back to what I had originally done. I wanted to cut his throat while I had to made him understand the code. After some 10 days,when I asked that it is ready to be merged,he says "I don't understand this part of code". Fucking bastard if you didn't understand,then why the fuck were you reviewing mine? The people who just did beginner changes but were from October got selected. This org. doesn't check your ability to resolve issues and understand code,but basically wants more number of commits,whether the commit may be mere change in documentation or so, doesn't matter. Again,these people want to help and reviewed my pr,but there should a valid argument. They meaninglessly just wanted to add their name to reviewers for making their proposal strong without helping or say by just showing off. I wrote unit tests, doctests, wrote a full-fledged function, resolved many PRs,and was working alone on one pr which was for the main release of SymPy,but I didn't get selected. Why? Because I started contributing in March. When will these guys understand what matters is how much you contribute not when you start to contribute. The substance and difficulty level of PRs should be considered not just no. of PRs. Hope this org. becomes more beginner friendly and open to more clear discussions rather than showing off.
☮️
Thanks.

Ugh. Been working on a huge React component that's now dependant on another co-workers PR, and had this one open for like a week. Go to merge and one of the fucking useless reviewers decides that *now* is the best time to flag everything wrong with my code!

I get it, it's good feedback, but uh... Could you not have done this a FUCKING WEEK AGO instead of RIGHT BEFORE I GO TO MERGE?!

Prick.

Our style check expects a default constructor. A new reviewers probably does not know it ...

Writting some documentation and asked for feedback, soon I felt cheated since "suggestions" were:
1 - Hey, this isn't implemented yet! I'm cutting it out...
2 - Hey! You shouldn't say this is how it works, we were in meeting and discussed that this is how it's gonna be, you need to change it...

Single reviewers, please consider consistency, thank you very much, bye