Code Reviews.
-
How do you guys do them?
cheers, Chris Maunder The Code Project | Co-founder Microsoft C++ MVP
Yes. They are mandatory for each unit of work here. The old "system" was just to pick on someone to review and get their comments. We were using a tool called Crucible[^], the UI was a bit unintuitive, but better than the alternatives + it integrates with SVN nicely, to you just open it up and review. Currently we are back to the old system of just finding a random punter to do it, apparently we are looking for a replacement to crucible (we were on a free trial). Before I came here I was against reviews, now I've moved round a bit to a more positive anything more than a sanity check is a waste of time as I'm less likely to wrap my head around the problem a fellow dev has spent time and effort working out. Still getting used to it though, so mod through inexperience on my part.
Sort of a cross between Lawrence of Arabia and Dilbert.[^]
-Or-
A Dead ringer for Kate Winslett[^] -
How do you guys do them?
cheers, Chris Maunder The Code Project | Co-founder Microsoft C++ MVP
As a less frivolous answer, our code reviews are done on an ad-hoc basis, about once a week, and more formally once a month. All code is fair game - my code is as up for review as the newest member of staff. We have a review checklist but, to be honest, we tend to pay more attention to results out of our CI process (where we run FxCop and StyleCop - even though I don't like them), plus NDepend and NCover.
I was brought up to respect my elders. I don't respect many people nowadays.
CodeStash - Online Snippet Management | My blog | MoXAML PowerToys | Mole 2010 - debugging made easier -
How do you guys do them?
cheers, Chris Maunder The Code Project | Co-founder Microsoft C++ MVP
Boss: I just let (Bob) go; we'd better take a look at what he produced, if anything.
-
How do you guys do them?
cheers, Chris Maunder The Code Project | Co-founder Microsoft C++ MVP
Whenever necessary, at least every stable release (Version number X.0.0.0). Rules: Program must work, object orientated programmed and code conventions must have be followed.
cheers Marco Bertschi
Software Developer & Founder SMGT Web-Portal CP Profile | My Articles | Twitter | Facebook | SMGT Web-Portal
-
Yes. They are mandatory for each unit of work here. The old "system" was just to pick on someone to review and get their comments. We were using a tool called Crucible[^], the UI was a bit unintuitive, but better than the alternatives + it integrates with SVN nicely, to you just open it up and review. Currently we are back to the old system of just finding a random punter to do it, apparently we are looking for a replacement to crucible (we were on a free trial). Before I came here I was against reviews, now I've moved round a bit to a more positive anything more than a sanity check is a waste of time as I'm less likely to wrap my head around the problem a fellow dev has spent time and effort working out. Still getting used to it though, so mod through inexperience on my part.
Sort of a cross between Lawrence of Arabia and Dilbert.[^]
-Or-
A Dead ringer for Kate Winslett[^] -
How do you guys do them?
cheers, Chris Maunder The Code Project | Co-founder Microsoft C++ MVP
Three levels: - pass-through: people have to go over the code and send results per mail, everybody on his own. - f2f: everybody sits in a room, odd number of people to get majority if questions, author presents his changes and everybody ... well, reviews. - inspection: (for critical code) just as f2f but with someone conducting the review who is not from the same department as the people authoring it. Findings documented in review minutes, on the fly corrections not allowed.
~RaGE();
I think words like 'destiny' are a way of trying to find order where none exists. - Christian Graus Do not feed the troll ! - Common proverb
-
How do you guys do them?
cheers, Chris Maunder The Code Project | Co-founder Microsoft C++ MVP
I look at it and go "Yeah! That's good code!" One person reviews don't generally catch much... ;)
If you get an email telling you that you can catch Swine Flu from tinned pork then just delete it. It's Spam.
-
Keith Barrow wrote:
Yes
- How do you answer an open question ? - Yes. :rolleyes:
~RaGE();
I think words like 'destiny' are a way of trying to find order where none exists. - Christian Graus Do not feed the troll ! - Common proverb
Manifesto: stop the posting of unhelpful comments pointing out that someone has "got something wrong" where the "something wrong" part is relatively minor, thus causing distraction. Such examples examples include spelling/syntactical/grammatical mistakes where is it pretty easy to work the originator's intention. Why "unhelpful comment"? At best the comment make the poster feel superior to someone else whilst denigrating or undermining the original poster especially when the originator is trying to add something to the debate, at worst it diverts the discussion away from the topic. Some people seem to mistake the Internet for an academic paper. -------------------- If you agree with the sentiment, please feel free to copy and paste this manifesto into your replies, including this comment -------------------- In this case, I misread CM's question as "Do you Guys do them"
Sort of a cross between Lawrence of Arabia and Dilbert.[^]
-Or-
A Dead ringer for Kate Winslett[^] -
How do you guys do them?
cheers, Chris Maunder The Code Project | Co-founder Microsoft C++ MVP
- Formalize them. If they are not formalized, eventually the review process will fade away 2) During the review focus on Code, Logic, and Design; ignore formatting, style, unless such style introduces an error. I could go on for months on this subject but, let's just say every code review I have participated in that was not run by me, focused on Style, Formatting and Coding Standards without a single minute spent on actual errors in code; which do you think is more important, honestly? Style formatting and Standards is a holy war with no winner; logic, functionality, approach and requirements all have the ability to be independently assessed and decrease code entropy. With that said, don't allow some college kid with no sense of style to randomly assign variable names and mix case depending on the mood, consistency per developer is more important than consistency on a team. 3) Don't focus on the easy stuff, if you do it becomes about nits. 4) If you find a teaching moment, use it. Bring in the team. Last year I found a code section that crashed production because of the as operator instead of casting. They had to bring me in to find the bug. 5) Allow yourself to be wrong.
Need custom software developed? I do custom programming based primarily on MS tools with an emphasis on C# development and consulting. "And they, since they Were not the one dead, turned to their affairs" -- Robert Frost "All users always want Excel" --Ennis Lynch
-
I look at it and go "Yeah! That's good code!" One person reviews don't generally catch much... ;)
If you get an email telling you that you can catch Swine Flu from tinned pork then just delete it. It's Spam.
-
Manifesto: stop the posting of unhelpful comments pointing out that someone has "got something wrong" where the "something wrong" part is relatively minor, thus causing distraction. Such examples examples include spelling/syntactical/grammatical mistakes where is it pretty easy to work the originator's intention. Why "unhelpful comment"? At best the comment make the poster feel superior to someone else whilst denigrating or undermining the original poster especially when the originator is trying to add something to the debate, at worst it diverts the discussion away from the topic. Some people seem to mistake the Internet for an academic paper. -------------------- If you agree with the sentiment, please feel free to copy and paste this manifesto into your replies, including this comment -------------------- In this case, I misread CM's question as "Do you Guys do them"
Sort of a cross between Lawrence of Arabia and Dilbert.[^]
-Or-
A Dead ringer for Kate Winslett[^]I agree, except where the error is Ironic, Amusing or of such vast incomprehensibleness as to render the reader speechless and/or catatonic.
--------------------------------- I will never again mention that I was the poster of the One Millionth Lounge Post, nor that it was complete drivel. Dalek Dave CCC Link[^]
-
How do you guys do them?
cheers, Chris Maunder The Code Project | Co-founder Microsoft C++ MVP
Just at this moment I'm managing a new project: We want to define a new QS-Process and for Code Review we want to use a tool. We are testing Jenkins-Server now.
------------------------------ Author of Primary ROleplaying SysTem How do I take my coffee? Black as midnight on a moonless night. War doesn't determine who's right. War determines who's left.
-
Manifesto: stop the posting of unhelpful comments pointing out that someone has "got something wrong" where the "something wrong" part is relatively minor, thus causing distraction. Such examples examples include spelling/syntactical/grammatical mistakes where is it pretty easy to work the originator's intention. Why "unhelpful comment"? At best the comment make the poster feel superior to someone else whilst denigrating or undermining the original poster especially when the originator is trying to add something to the debate, at worst it diverts the discussion away from the topic. Some people seem to mistake the Internet for an academic paper. -------------------- If you agree with the sentiment, please feel free to copy and paste this manifesto into your replies, including this comment -------------------- In this case, I misread CM's question as "Do you Guys do them"
Sort of a cross between Lawrence of Arabia and Dilbert.[^]
-Or-
A Dead ringer for Kate Winslett[^]Historical background: (which I should have explained right away) I answer all my wife's open question with "yes". And find it funny. Sorry for making insider jokes with myself. :) OTOH, that "Yes" sitting alone, nicely separated from the rest of the text, was yielding for a comment. As for the manifesto, you are right, I now feel superior to you and denigrate you, especially since you brought something valuable to the debate. My hobby is to make the Internet a mistake free zone ( but for the ones I am making myself). ;P
~RaGE();
I think words like 'destiny' are a way of trying to find order where none exists. - Christian Graus Do not feed the troll ! - Common proverb
-
I agree, except where the error is Ironic, Amusing or of such vast incomprehensibleness as to render the reader speechless and/or catatonic.
--------------------------------- I will never again mention that I was the poster of the One Millionth Lounge Post, nor that it was complete drivel. Dalek Dave CCC Link[^]
-
How do you guys do them?
cheers, Chris Maunder The Code Project | Co-founder Microsoft C++ MVP
Not sure if you are familiar, but in TFS2012 there is some really cool options. Basically when a junior grunt needs to check something in, he requests a code review (then he goes to the bathroom or something). I get an alert, when I have time (more often I'd rather just get it over), I click "Yes, I accept your code review request". I then look at his changes to the server version, adding comments to code when needed (Now this is where TFS2012 is awesome, the comments don't actually go into the code, but into the code review and have a corresponding section of code, super cool). I typically don't review it in front of him, because most of the time I don't need to. I also review each check in to any shared code, so it happens often. If it's his own project, I'll review periodically to see if the architecture is FUBARed so he doesn't waste time going down a bad path. Now, sometimes, if the review to shared code requires a complex explanation, I call him over to my desk and apply proper punishment show him the errors of his ways, all the while adding comments as I bring up my points. Then I send it back to him to try again.
-
If you're even half good, you place a hand on the machine and feel the code...
Reality is an illusion caused by a lack of alcohol
Sounds like this story could get dirty. :-D
-
How do you guys do them?
cheers, Chris Maunder The Code Project | Co-founder Microsoft C++ MVP
Since I work alone, my code reviews are pretty simple. If it's been less than 1 day since I wrote the code, I think it is the most brilliant code ever written. If it's been more than a day, then I write comments about the author have been insane and it all needs a rewrite whenever there is time - which there never is.
CQ de W5ALT
Walt Fair, Jr., P. E. Comport Computing Specializing in Technical Engineering Software
-
I've still got my dog, so I'm alright. He's just as reliable as he ever was.[^] :-\
That is the first time I have ever upvoted something over two years old...
If you get an email telling you that you can catch Swine Flu from tinned pork then just delete it. It's Spam.
-
Manifesto: stop the posting of unhelpful comments pointing out that someone has "got something wrong" where the "something wrong" part is relatively minor, thus causing distraction. Such examples examples include spelling/syntactical/grammatical mistakes where is it pretty easy to work the originator's intention. Why "unhelpful comment"? At best the comment make the poster feel superior to someone else whilst denigrating or undermining the original poster especially when the originator is trying to add something to the debate, at worst it diverts the discussion away from the topic. Some people seem to mistake the Internet for an academic paper. -------------------- If you agree with the sentiment, please feel free to copy and paste this manifesto into your replies, including this comment -------------------- In this case, I misread CM's question as "Do you Guys do them"
Sort of a cross between Lawrence of Arabia and Dilbert.[^]
-Or-
A Dead ringer for Kate Winslett[^]Keith Barrow wrote:
Some people seem to mistake the Internet for an academic paper
...and insist that informal discussions remain "on topic". ;) If you don't appreciate a reply, my advice it to just leave it without a further reply and let that branch die.
-
Keith Barrow wrote:
Some people seem to mistake the Internet for an academic paper
...and insist that informal discussions remain "on topic". ;) If you don't appreciate a reply, my advice it to just leave it without a further reply and let that branch die.
AspDotNetDev wrote:
...and insist that informal discussions remain "on topic
Nope, just want to reduce noise.
Sort of a cross between Lawrence of Arabia and Dilbert.[^]
-Or-
A Dead ringer for Kate Winslett[^]
