The reviewers for my PR proposed some code changes, which were supposed to be a “cleaner” way to do things. I was not super familiar with the proposed approach, but I followed them at face value and got approval for the PR.
It turned out the suggested changes were flawed / incomplete and required additional handling of edge cases, whereas my original code did not need this.
Fast forward to the code break in production. It looked very bad on me because the bug manifested itself in multiple incidents. Management also spoke to me many times and asked me to do more testing in the future.
My bonus was shit this year because of this, and I’ve been trying to digest all this for a while. I understand I should be responsible since it’s my PR, but I just don’t think I should be the only one to blame. I’m already doing the most testing on my team, but it’s impossible to cover the edge cases if they are not known. I’m happy to learn from PR suggestions, but should the reviewer be mindful of their code comments, if they are suggesting something new to someone and approving when what they are suggesting is flawed/incomplete?
I know all of this wouldn’t have happened if I was experienced enough to know the flaw in the code suggestions, but I just wanted to let it out of my chest.
submitted by /u/lampinwater
[link] [comments]
r/cscareerquestions The reviewers for my PR proposed some code changes, which were supposed to be a “cleaner” way to do things. I was not super familiar with the proposed approach, but I followed them at face value and got approval for the PR. It turned out the suggested changes were flawed / incomplete and required additional handling of edge cases, whereas my original code did not need this. Fast forward to the code break in production. It looked very bad on me because the bug manifested itself in multiple incidents. Management also spoke to me many times and asked me to do more testing in the future. My bonus was shit this year because of this, and I’ve been trying to digest all this for a while. I understand I should be responsible since it’s my PR, but I just don’t think I should be the only one to blame. I’m already doing the most testing on my team, but it’s impossible to cover the edge cases if they are not known. I’m happy to learn from PR suggestions, but should the reviewer be mindful of their code comments, if they are suggesting something new to someone and approving when what they are suggesting is flawed/incomplete? I know all of this wouldn’t have happened if I was experienced enough to know the flaw in the code suggestions, but I just wanted to let it out of my chest. submitted by /u/lampinwater [link] [comments]
The reviewers for my PR proposed some code changes, which were supposed to be a “cleaner” way to do things. I was not super familiar with the proposed approach, but I followed them at face value and got approval for the PR.
It turned out the suggested changes were flawed / incomplete and required additional handling of edge cases, whereas my original code did not need this.
Fast forward to the code break in production. It looked very bad on me because the bug manifested itself in multiple incidents. Management also spoke to me many times and asked me to do more testing in the future.
My bonus was shit this year because of this, and I’ve been trying to digest all this for a while. I understand I should be responsible since it’s my PR, but I just don’t think I should be the only one to blame. I’m already doing the most testing on my team, but it’s impossible to cover the edge cases if they are not known. I’m happy to learn from PR suggestions, but should the reviewer be mindful of their code comments, if they are suggesting something new to someone and approving when what they are suggesting is flawed/incomplete?
I know all of this wouldn’t have happened if I was experienced enough to know the flaw in the code suggestions, but I just wanted to let it out of my chest.
submitted by /u/lampinwater
[link] [comments]