Skip to main content
Active reading. [<http://www.wikihow.com/Use-You%27re-and-Your> <http://www.wikihow.com/Use-Than-and-Then> <https://en.wiktionary.org/wiki/non-deterministic#Adjective> <https://en.wiktionary.org/wiki/self-evident#Adjective>].
Source Link

Code can ALWAYS be better.

If youryou're in a code review and you don't see anything that might be better or a unit test that might catch a bug its, it's not the code thatsthat's perfect, but the reviewer that isn't doing their job. Whether you chose to mention the improvement is a personal choice. But almost any time your team does a Code Review code review there should be things that someone notices that could be better or everyone likely just wasted their time.

ThisThat said, whether you act on the comments or not is up to your team. If your changes fix the issue or adds enough value without change that your team accepts them then merge them in and log their comments into the backlog for someone to address later. If the TEAMteam finds your changes add more risk or complexity thenthan value then you have to resolve the issues accordingly.

Just remember that any code has at least one more edge case that could be tested and could use at least 1one more refactorrefactoring. This is why code reviews are best done as a group with everyone looking at the same code at the same time. So that at the end everyone can come to a consensus on whether or not the code in review is acceptable (as it is) and adds enough value to merge into the community base or if certain things must be done before there is enough value to merge.

Since you are asking this question I assume youryou're not actually doing "Code Reviews""code reviews", but instead creating a pull request or other submission mechanism for others to comment in a non deterministic-deterministic way. So now youryou're into a management issue and a definition of done. I'd guess your management is indecisive and doesn't actually understand the process and purpose of code reviews and likely doesn't have a "Definition Of Done""definition of done" (DOD). Because if they did your DOD would explicitly answer this question and you wouldn't have to come here and askasked.

How do you fix it.? Well, ask you manager to give you a DOD and tell you if you have to always implement all comments. If the person in question is your manager then the answer is self obvious-evident.

Code can ALWAYS be better.

If your in a code review and you don't see anything that might be better or a unit test that might catch a bug its not the code thats perfect but the reviewer that isn't doing their job. Whether you chose to mention the improvement is a personal choice. But almost any time your team does a Code Review there should be things that someone notices that could be better or everyone likely just wasted their time.

This said whether you act on the comments or not is up to your team. If your changes fix the issue or adds enough value without change that your team accepts them then merge them in and log their comments into the backlog for someone to address later. If the TEAM finds your changes add more risk or complexity then value then you have to resolve the issues accordingly.

Just remember that any code has at least one more edge case that could be tested and could use at least 1 more refactor. This is why code reviews are best done as a group with everyone looking at the same code at the same time. So that at the end everyone can come to a consensus on whether or not the code in review is acceptable (as it is) and adds enough value to merge into the community base or if certain things must be done before there is enough value to merge.

Since you are asking this question I assume your not actually doing "Code Reviews" but instead creating a pull request or other submission mechanism for others to comment in a non deterministic way. So now your into a management issue and a definition of done. I'd guess your management is indecisive doesn't actually understand the process and purpose of code reviews and likely doesn't have a "Definition Of Done". Because if they did your DOD would explicitly answer this question and you wouldn't have to come here and ask.

How do you fix it. Well ask you manager to give you a DOD and tell you if you have to always implement all comments. If the person in question is your manager then the answer is self obvious.

Code can ALWAYS be better.

If you're in a code review and you don't see anything that might be better or a unit test that might catch a bug, it's not the code that's perfect, but the reviewer that isn't doing their job. Whether you chose to mention the improvement is a personal choice. But almost any time your team does a code review there should be things that someone notices that could be better or everyone likely just wasted their time.

That said, whether you act on the comments or not is up to your team. If your changes fix the issue or adds enough value without change that your team accepts them then merge them in and log their comments into the backlog for someone to address later. If the team finds your changes add more risk or complexity than value then you have to resolve the issues accordingly.

Just remember that any code has at least one more edge case that could be tested and could use at least one more refactoring. This is why code reviews are best done as a group with everyone looking at the same code at the same time. So that at the end everyone can come to a consensus on whether or not the code in review is acceptable (as it is) and adds enough value to merge into the community base or if certain things must be done before there is enough value to merge.

Since you are asking this question I assume you're not actually doing "code reviews", but instead creating a pull request or other submission mechanism for others to comment in a non-deterministic way. So now you're into a management issue and a definition of done. I'd guess your management is indecisive and doesn't actually understand the process and purpose of code reviews and likely doesn't have a "definition of done" (DOD). Because if they did your DOD would explicitly answer this question and you wouldn't have to come here and asked.

How do you fix it? Well, ask you manager to give you a DOD and tell you if you have to always implement all comments. If the person in question is your manager then the answer is self-evident.

Source Link

Code can ALWAYS be better.

If your in a code review and you don't see anything that might be better or a unit test that might catch a bug its not the code thats perfect but the reviewer that isn't doing their job. Whether you chose to mention the improvement is a personal choice. But almost any time your team does a Code Review there should be things that someone notices that could be better or everyone likely just wasted their time.

This said whether you act on the comments or not is up to your team. If your changes fix the issue or adds enough value without change that your team accepts them then merge them in and log their comments into the backlog for someone to address later. If the TEAM finds your changes add more risk or complexity then value then you have to resolve the issues accordingly.

Just remember that any code has at least one more edge case that could be tested and could use at least 1 more refactor. This is why code reviews are best done as a group with everyone looking at the same code at the same time. So that at the end everyone can come to a consensus on whether or not the code in review is acceptable (as it is) and adds enough value to merge into the community base or if certain things must be done before there is enough value to merge.

Since you are asking this question I assume your not actually doing "Code Reviews" but instead creating a pull request or other submission mechanism for others to comment in a non deterministic way. So now your into a management issue and a definition of done. I'd guess your management is indecisive doesn't actually understand the process and purpose of code reviews and likely doesn't have a "Definition Of Done". Because if they did your DOD would explicitly answer this question and you wouldn't have to come here and ask.

How do you fix it. Well ask you manager to give you a DOD and tell you if you have to always implement all comments. If the person in question is your manager then the answer is self obvious.