16
Jifuna
2y

It's rant time again. I was working on a project which exports data to a zipped csv and uploads it to s3. I asked colleagues to review it, I guess that was a mistake.

Well, two of my lesser known colleague reviewed it and one of the complaints they had is that it wasn't typescript. Well yes good thing you have EYES, i'm not comfortable with typescript yet so I made it in nodejs (which is absolutely fine)

The other guy said that I could stream to the zip file and which I didn't know was possible so I said that's impossible right? (I didn't know some zip algorithms work on streams). And he kept brushing over it and taking about why I should use streams and why. I obviously have used streams before and if had read my code he could see that my code streamed everything to the filesystem and afterwards to s3. He continued to behave like I was a literall child who just used nodejs for 2 seconds. (I'm probably half his age so fair enough). He also assumed that my code would store everything in memory which also isn't true if he had read my code...

Never got an answer out of him and had to google myself and research how zlib works while he was sending me obvious examples how streams work. Which annoyed me because I asked him a very simple question.

Now the worst part, we had a dev meeting and both colleagues started talking about how they want that solutions are checked and talked about beforehand while talking about my project as if it was a failure. But it literally wasn't lol, i use streams for everything except the zipping part myself because I didn't know that was possible.

I was super motivated for this project but fuck this shit, I'm not sure why it annoys me so much. I wanted good feedback not people assuming because I'm young I can't fucking read documentation and also hate that they brought it up specifically pointing to my project, could be a general thing. Fuck me.

Comments
  • 7
    Ouch, tough luck my man. But stay strong and confident 😀 just sounds like two devs trying to show off their own skills. Say they are good solutions, but you chose a path that would be more optimal for you (=less dev hours and thus cheaper)
  • 1
    a good CR should be a positive learning experience. But it can be really bad, like what heppend here.

    When the reviewer can't explain the issues (unproffesional idiot), Those who do not give rational reasons (primadona dev/ego problems), and those who say the code is bad to everyone else (credit steal/ladder climb).
    The dev under review sometime tend to overly defend thier work. Never do that - code that works, and passes the tests should be merged. Code that doesn't should not be reviewed at all. In case the problems are huge - say O(2^n) time complexity, then all improvments should be in the next iteration.

    CR should be used to learn, and avoid major problems. Not used by code nazis that think they are better the code linter, or discourage juniors from coding. They should also be private. Talk to your seniors, and tell them the process was a failure!
  • 2
    I had some random person from the other team reviewing my PR. Commenting on parts of the code that weren't part of the PR.

    Kinda half valid comments (apart from a syntax complain where I was clearly right, it's TS/JS sooo it is a bit weird if you're not familiar with it) IF IT WASN'T A BUG FIX THAT NEEDED TO BE MERGED ASAP.

    Grrrrrrrrrrrrr.
Add Comment