|
I am quite keen on doing code reviews at my current place of employment - although I have not worked anywhere that does code reviews before.
My belief is that code reviews will help ensure standards are kept, enhance the learning of all the developers, and help drive any changes in standards.
The other developers think it is a waste of time except for new developers having their code reviewed (presumably until they can be trusted)
Your thoughts/experiences/advice?
As a blatant gamble for more points, vote this message up if you think Code reviews are a Good Thing, and down if you think they are the devil's work!
MVVM # - I did it My Way
___________________________________________
Man, you're a god. - walterhevedeich 26/05/2011
.\\axxx
(That's an 'M')
|
|
|
|
|
|
tldr
MVVM # - I did it My Way
___________________________________________
Man, you're a god. - walterhevedeich 26/05/2011
.\\axxx
(That's an 'M')
|
|
|
|
|
+1
Code review is one of the hardest things to do and the most valuable.
Most of the time the code reviewed is really simple and you just have to nit-pik; but sometimes you get the "mother-load" and get some code that you really are not familiar with and then you spend a lot of time just understanding the "domain" and the algorithms before digging in the actual code.
I'd rather be phishing!
|
|
|
|
|
Reviewing other people's code is fun to an extent exponentially related to the size of their ego and organizational title.
yours, Bill
"What Turing gave us for the first time (and without Turing you just couldn't do any of this) is he gave us a way of thinking about and taking seriously and thinking in a disciplined way about phenomena that have, as I like to say, trillions of moving parts.
Until the late 20th century, nobody knew how to take seriously a machine with a trillion moving parts. It's just mind-boggling." Daniel C. Dennett
|
|
|
|
|
I hear you!
Already getting the "my code doesn't need to be reviewed" from a couple!
MVVM # - I did it My Way
___________________________________________
Man, you're a god. - walterhevedeich 26/05/2011
.\\axxx
(That's an 'M')
|
|
|
|
|
Spot on 
“That which can be asserted without evidence, can be dismissed without evidence.”
― Christopher Hitchens
|
|
|
|
|
In general I have good experience with code reviews - not as much as a tool for discovering bug as a mechanism for sharing knowledge.
However, as with most things in software engineering, you shouldn't go overboard with them. I've been in situations where code reviews were taking ridiculous amount of time and energy. In most cases, a code review should take one round of comments and responses, and the committer should be allowed to disregard the comments unless he/she is a junior developer.
|
|
|
|
|
I can't agree more.
Sadly I have been mostly at companies which did it wrong and were code reviews took days. They actually didn't code reviews, they looked at how they should have built it and then tell you you did it wrong. If you didn't comply with the architecture, that wouldn't have been a problem ofcourse, but the code always complied.
The there-are-multiple-ways-to-Rome idea missed its road from their building to Rome, because they really didn't get it and always complained they didn't have enough time to do their work, because they had to review everything...
A good code review looks for any problems, any deviations from required standards and some tips/hints. You should clearly separate those groups of comments. Problems and deviations need to be fixed. Tips/hints should be considered helpful and not a kick in the butt.
|
|
|
|
|
Nemanja Trifunovic wrote: and the committer should be allowed to disregard the comments
Review ticket items should be categorized as one of the following
- security bug
- bug
- comment
- well done
Security bugs must be fixed before the code goes to production. And it must be reviewed for the fix.
Bugs can be triaged but with the goal to fix.
Comments are suggestions about alternate ways to do something. There need not be any action at all.
Well done (or create your own term) is an indication that the reviewer really likes the way a piece of code works.
|
|
|
|
|
I've been a developer for several decades. I get value from my code being reviewed by others.
My ideas on reviewing of other people's code should be obvious.
====
On a related note, one of my observations is that every contributor will search until they find something to criticize. One of my review techniques (for my code submissions) is to make sure that there is at least one blatant mistake that everybody can find, which can be "fixed" with a trivial action on my part; else if your submission is truly close to perfect, you will get several new requirements to add wheel spinning features that have little (or negative) value.
Windows 8 is the resurrected version of Microsoft Bob. The only thing missing is the Fisher-Price logo.
- Harvey
|
|
|
|
|
A few of my thoughts:
1) Code reviews are good.
2) If you think your code doesn't need to be reviewed - Then it needs to be reviewed
3) As you said, code reviews are for everyone, Say you write perfect code, what better way for someone else to learn something they didnt know of
4) More and more, static analysis tools can check for coding standards, unfortanately most of the comments in reviews relate to naming conventions and comments :/
5) I like to focus on correctness of code and design for integration with the system and future maintainability.
To know and not do, is not yet to know
http://www.codeofthedamned.com
|
|
|
|
|
What a silly idea. I'm a man, I know what I am doing and there is no reason for anyone else to look at my perfect code. I've never made a mistake and I never will.
|
|
|
|
|
You're married... when was the last time you were right about anything???  
|
|
|
|
|
That is not related to marriage. I know this because I'm in fact not married but living in sin and still incorrect about everything.
|
|
|
|
|
|
Given the amount I've heard lately about her preferences in diamond settings I suspect she would prefer tomato
|
|
|
|
|
You're getting the hint then? Wait, are you currently in custody up here at Indooroopilly??? (If this makes no sense, google Indooroopilly diamond theft.)
|
|
|
|
|
_Damian S_ wrote: You're getting the hint then? Wait, are you currently in custody up here at Indooroopilly??? (If this makes no sense, google Indooroopilly diamond theft.)
Josh would need several layers of sun tan to be the diamond ring pooper man.
Michael Martin
Australia
"I controlled my laughter and simple said "No,I am very busy,so I can't write any code for you". The moment they heard this all the smiling face turned into a sad looking face and one of them farted. So I had to leave the place as soon as possible."
- Mr.Prakash One Fine Saturday. 24/04/2004
|
|
|
|
|
True that... maybe it was his brilliant disguise?
|
|
|
|
|
_Damian S_ wrote: (If this makes no sense, google Indooroopilly diamond theft.)
Sorry, I was in the bathroom with constable smellmyturd
|
|
|
|
|
Get back into the kitchen with the other girlies
MVVM # - I did it My Way
___________________________________________
Man, you're a god. - walterhevedeich 26/05/2011
.\\axxx
(That's an 'M')
|
|
|
|
|
But seriously, either your workmates think like this or they are scared of being shown up by the new guys
|
|
|
|
|
A little of both, I think.
Many closed minds.
MVVM # - I did it My Way
___________________________________________
Man, you're a god. - walterhevedeich 26/05/2011
.\\axxx
(That's an 'M')
|
|
|
|
|
I've had excellent experience with Code Reviews.
We have found a number of bugs that would have gone unnoticed if not for code reviews, and I'm a strong advocate of having code reviews.
"Real men drive manual transmission" - Rajesh.
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
