OSDev.org
https://forum.osdev.org/

Effective Code Reviews
https://forum.osdev.org/viewtopic.php?f=13&t=33578
Page 1 of 1

Author:  Thomas [ Tue Mar 12, 2019 11:13 am ]
Post subject:  Effective Code Reviews

What steps do you do get good review comments from reviewers ? I am just curious to see how other engineers go about it ? When i submit code for review, i want other engineers to engage creatively about how the code can break not about "how their way of doing it is better than others" or a misplaced indentation in comments which has absolutely no effect on product. I eventually end up finding the possible flaws myself and review just seems to have no good effect at times.


--Thomas

Author:  Combuster [ Tue Mar 12, 2019 11:47 am ]
Post subject:  Re: Effective Code Reviews

Companies keep standards. Not because the standard is the best, but because it makes things predictable and to allow anybody on the team to deal with your code as efficiently as anybody's. Don't confuse it with egocentrism.
Things like that are also much easier on people compared to mentally doing a full data flow analysis on your submitted code, which effectively means anybody can review your indentation, but not everybody has the mindset to effectively notice all the corner cases and antipatterns that might be present. And of course, there's quite often pressure on people to finish their thing, typically at the cost of your thing.

The one remaining legitimate issue here seems to be that bugs get missed in review. For that one, I hope you are skilled in the workplace art of discussing problems without becoming confrontational.

Author:  SpyderTL [ Tue Mar 12, 2019 12:33 pm ]
Post subject:  Re: Effective Code Reviews

If you are reviewing someone's code, just be clear about the nature of any issues that you discover. Minor issues, or personal preferences where you would have taken a different approach are fine, as long as everyone is clear that a) the code does not necessarily need to be changed, and b) you explain, briefly, why you would have done it a different way. (i.e. Aesthetics, performance, simplicity, readability, etc.)

On the flip side, if someone is reviewing your code, be sure you understand where the reviewer is coming from in regards to the items above. If necessary, ask specifically whether their issue is more of a personal preference, or if it is something that needs to be addressed.

Everyone involved just needs to realize that no two developers will ever agree on a particular approach, so unless you have made some sort of team agreement, or the company or client has mandated some sort of guidelines, all solutions should be considered acceptable as long as the end result meets the expected behavior.

It's hard to look at a block of code that is new or that has been changed by someone else, and spot logic errors in a 5 minute code review. Which is why we've moved to pair programming, or mob programming with up to 5 people in a conference room working on a single task. It is very unlikely that you will have a typo (= instead of ==), or a logic error (!= instead of ==), or forget a critical step when other people are watching. As soon as you deviate from their expected path, they will immediately start making noise. Another advantage is that once you are done, you will have 2-5 people who understand how the code works, rather than 1-2. However, this approach can be overkill for simple modifications, like label changes, or hiding/removing UI elements. But it does virtually eliminate the need for code reviews...

Author:  Thomas [ Tue Mar 12, 2019 3:22 pm ]
Post subject:  Re: Effective Code Reviews

Code:
Companies keep standards. Not because the standard is the best, but because it makes things predictable and to allow anybody on the team to deal with your code as efficiently as anybody's. Don't confuse it with egocentrism.

Ideally this would be true. There are global coding standards. But an owner of a particular module might do it differently going against the existing standards and i fit my style to mimic it. My gripe is that comments should not be only about coding standards alone which are appreciated.

Code:
there's quite often pressure on people to finish their thing, typically at the cost of your thing.

This is a real problem and to be fair it is not possible to catch everything in a review. However, I get the feeling that just looking at the coding standards is an easy way out!.

Most people are motivated and intelligent. I was thinking how we can tap into that without making it a lot of work.

--Thomas

Author:  Thomas [ Tue Mar 12, 2019 3:26 pm ]
Post subject:  Re: Effective Code Reviews

Quote:
Which is why we've moved to pair programming, or mob programming with up to 5 people in a conference room working on a single task. It is very unlikely that you will have a typo (= instead of ==), or a logic error (!= instead of ==), or forget a critical step when other people are watching. As soon as you deviate from their expected path, they will immediately start making noise. Another advantage is that once you are done, you will have 2-5 people who understand how the code works, rather than 1-2. However, this approach can be overkill for simple modifications, like label changes, or hiding/removing UI elements. But it does virtually eliminate the need for code reviews...


Interesting reply. Thank you!.

Page 1 of 1 All times are UTC - 6 hours
Powered by phpBB © 2000, 2002, 2005, 2007 phpBB Group
http://www.phpbb.com/