Ask HN: How to learn from code reviews?

[Sherxon9]

I have more than 4 years of experience in software development and I joined a new company as a software engineer 3 months ago.

I love learning and getting feedback from others.But when 9 out of 10 commits are reviewed and rejected by the same Sr. Software engineer in my team I felt like there is smth wrong with me or with the other person. Recently, I ignored some of his feedback to change my code :( (i know it wasn't good)

My Question is How to deal with such situations?

[techjuice]

As a junior engineer with only 4 years of experience it is best to take all feedback good and bad to analyze what you may or may not be doing that is not receiving approval acceptable to make it in the production build.

Are you following code style guidelines, is your code secure, is your code fast, are you make large code pushes that takes too long to review and test, are you submitting tests when required, are you updating your documentation, did you follow spec, does your code solve a problem related to a feature, bug request, etc.? If you are then there could be other things that might be outside of what you are doing that are not being properly communicated to you at all.

Have you talked to the team lead about the issues in your code to work towards improving the code you commit? Though it would be very beneficial to have the capability to submit your code to a dev CI/CD system that tells you what is wrong with your code automatically before you submit it for production review.

Though ultimately if you are not receiving feedback on how to improve the code your committing you should bring it up with management as it is the senior and above job to help guide and mentor junior developer's or new team members with any issues in their code. If this is not being done then it could be a leadership, managerial or cultural problem that needs to be resolved early on.

If you are still getting large rejections could you give us a few anonymized samples of some of the rejection notes on your commits?

[Sherxon9]

thank you for your comment, yes code formatting, guidelines are good. I havent talked to team lead yet. I talked to the Sr engineer several times, and often times he comes up with different approach to the problem and ask me to implement his solution even if my version has passed all test cases) it makes me sad

[thisone]

If there has been no explanation for a requested change, ask for one. A reviewer should never ask for changes for change's sake, they need to have a reason. So ask for that reason, then you can decide whether to push back or not, or ask for further clarification.

[yoklov]

If they’re suggesting better or even equivalent ways, I just do it. If you are not sure, also just do it? Life’s too short to be arguing about every code review comment.

The only time I argue about review comments is if i think it wont work, or if I think it should be addressed in a future bug (maybe also if I think its much more complexity).

In general I don’t think you should treat it as saying really anything about you. They’re comments about the code, not about you or even your skill as a programmer.

[eberkund]

I agree with your statement that if 9 out of 10 commits are being rejected something is seriously wrong and that is definitely not a productive use of time for either of you. What is his reasoning for rejecting your code?

Is it nitpicking over variable names or spacing? Performance issues? Structure?

[Sherxon9]

thank you for your comment. It is not nitpicking :) I can say that it is mostly about different approaches to the problem.