Is the Code Review Parallel Programming or Singleton?
-
I have a new team forming up and we want to decide on the best way to formalize code reviews. Is the Code Review the responsibility of one person or the whole team? Is each, or certain, team member(s) allowed to review others' code or is there only one person who does the reviews? If it's only one person, then who reviews his/her code? I've heard of both in practice and some of the teams where I'm at use both patterns. I'm just looking for insight as to what everyone thinks is the best way to go and why.
- Arcond
-
I have a new team forming up and we want to decide on the best way to formalize code reviews. Is the Code Review the responsibility of one person or the whole team? Is each, or certain, team member(s) allowed to review others' code or is there only one person who does the reviews? If it's only one person, then who reviews his/her code? I've heard of both in practice and some of the teams where I'm at use both patterns. I'm just looking for insight as to what everyone thinks is the best way to go and why.
- Arcond
42
CQ de W5ALT
Walt Fair, Jr., P. E. Comport Computing Specializing in Technical Engineering Software
-
42
CQ de W5ALT
Walt Fair, Jr., P. E. Comport Computing Specializing in Technical Engineering Software
-
Bacon!
"Whether you think you can, or you think you can't--either way, you are right." — Henry Ford "When I waste my time, I only use the best, Code Project...don't leave home without it." — Slacker007
Liquid Nitrogen!
Help a brotha out and vote Managing Your JavaScript Library in ASP.NET as the best ASP.NET article of May 2011.
-
Liquid Nitrogen!
Help a brotha out and vote Managing Your JavaScript Library in ASP.NET as the best ASP.NET article of May 2011.
-
I have a new team forming up and we want to decide on the best way to formalize code reviews. Is the Code Review the responsibility of one person or the whole team? Is each, or certain, team member(s) allowed to review others' code or is there only one person who does the reviews? If it's only one person, then who reviews his/her code? I've heard of both in practice and some of the teams where I'm at use both patterns. I'm just looking for insight as to what everyone thinks is the best way to go and why.
- Arcond
Set coding standards and guidelines and let everyone review everyone else's code. Unless it's NASA level code, then you would have a code review team.
If your actions inspire others to dream more, learn more, do more and become more, you are a leader." - John Quincy Adams
You must accept one of two basic premises: Either we are alone in the universe, or we are not alone in the universe. And either way, the implications are staggering” - Wernher von Braun -
I have a new team forming up and we want to decide on the best way to formalize code reviews. Is the Code Review the responsibility of one person or the whole team? Is each, or certain, team member(s) allowed to review others' code or is there only one person who does the reviews? If it's only one person, then who reviews his/her code? I've heard of both in practice and some of the teams where I'm at use both patterns. I'm just looking for insight as to what everyone thinks is the best way to go and why.
- Arcond
Senior people review less senior people's code. When equal level peers are dependent on each other's code, they may review each other. Saying something like "let everyone review everyone else's code" is a waste of time if, for example, an intern is reviewing an architects library, etc, unless it's for learning experience.
"I have a theory that the truth is never told during the nine-to-five hours. " — Hunter S. Thompson
-
A pig walked into a bar with a tank of liquid nitrogen and forty-two CListCtrls... Hmm... Nah, doesn't work.
Proud to have finally moved to the A-Ark. Which one are you in?
Author of the Guardians Saga (Sci-Fi/Fantasy novels) -
Liquid Nitrogen!
Help a brotha out and vote Managing Your JavaScript Library in ASP.NET as the best ASP.NET article of May 2011.
Balanced the 1 vote of doofus.
"Real men drive manual transmission" - Rajesh.
-
A pig walked into a bar with a tank of liquid nitrogen and forty-two CListCtrls... Hmm... Nah, doesn't work.
Proud to have finally moved to the A-Ark. Which one are you in?
Author of the Guardians Saga (Sci-Fi/Fantasy novels)And said "It's PORNO time" ... Yes it does. Kinda. In my head :)
Sort of a cross between Lawrence of Arabia and Dilbert.[^]
-Or-
A Dead ringer for Kate Winslett[^] -
Senior people review less senior people's code. When equal level peers are dependent on each other's code, they may review each other. Saying something like "let everyone review everyone else's code" is a waste of time if, for example, an intern is reviewing an architects library, etc, unless it's for learning experience.
"I have a theory that the truth is never told during the nine-to-five hours. " — Hunter S. Thompson
wizardzz wrote:
an intern is reviewing an architects library, etc, unless it's for learning experience.
This really depends on the place. My internships dealt with code that had never had a pure programmer work on it, everything was done by engineers who learned to code. While everything worked, some of it was a rather marvelous mess and a great deal of it was massively improved by the time another intern and I were done with it. It's probably rare, at least I'd like to think it is, but it would have been a lot easier to deal with had they actually allowed us to look at all of the code to see what could be overhauled when we started, rather than spending half our time working through half hacked together pages until we found consistent patterns we could correct. The only real advice I have for code reviews is to have people actually look at the code before the meeting. Doesn't have to be much, just enough to have a clue what's supposed to go on. The one official one I went through was remarkably painful compared to the more impromptu meetings that the other intern and I had setup because half the room had no clue what was even supposed to be going on. Though it also helps if they actually care. That was a bit of a problem with the official meeting as well, the code had remarkably well defined territories within it. But that's a culture problem.
-
I have a new team forming up and we want to decide on the best way to formalize code reviews. Is the Code Review the responsibility of one person or the whole team? Is each, or certain, team member(s) allowed to review others' code or is there only one person who does the reviews? If it's only one person, then who reviews his/her code? I've heard of both in practice and some of the teams where I'm at use both patterns. I'm just looking for insight as to what everyone thinks is the best way to go and why.
- Arcond
I've got plenty of time to do it just send the code and a check.
"Life can only be understood backwards, but it must be lived forward." Kierkegaard, Søren
-
42
CQ de W5ALT
Walt Fair, Jr., P. E. Comport Computing Specializing in Technical Engineering Software
52 is the new 42. :)
"Life can only be understood backwards, but it must be lived forward." Kierkegaard, Søren
-
I have a new team forming up and we want to decide on the best way to formalize code reviews. Is the Code Review the responsibility of one person or the whole team? Is each, or certain, team member(s) allowed to review others' code or is there only one person who does the reviews? If it's only one person, then who reviews his/her code? I've heard of both in practice and some of the teams where I'm at use both patterns. I'm just looking for insight as to what everyone thinks is the best way to go and why.
- Arcond
Quis custodiet ipsos custodes? That is indeed the question.
¡El diablo está en mis pantalones! ¡Mire, mire! SELECT * FROM User WHERE Clue > 0 0 rows returned Save an Orange - Use the VCF! Personal 3D projects Just Say No to Web 2 Point Blow
-
wizardzz wrote:
an intern is reviewing an architects library, etc, unless it's for learning experience.
This really depends on the place. My internships dealt with code that had never had a pure programmer work on it, everything was done by engineers who learned to code. While everything worked, some of it was a rather marvelous mess and a great deal of it was massively improved by the time another intern and I were done with it. It's probably rare, at least I'd like to think it is, but it would have been a lot easier to deal with had they actually allowed us to look at all of the code to see what could be overhauled when we started, rather than spending half our time working through half hacked together pages until we found consistent patterns we could correct. The only real advice I have for code reviews is to have people actually look at the code before the meeting. Doesn't have to be much, just enough to have a clue what's supposed to go on. The one official one I went through was remarkably painful compared to the more impromptu meetings that the other intern and I had setup because half the room had no clue what was even supposed to be going on. Though it also helps if they actually care. That was a bit of a problem with the official meeting as well, the code had remarkably well defined territories within it. But that's a culture problem.
Distind wrote:
The only real advice I have for code reviews is to have people actually look at the code before the meeting. Doesn't have to be much, just enough to have a clue what's supposed to go on. The one official one I went through was remarkably painful compared to the more impromptu meetings that the other intern and I had setup because half the room had no clue what was even supposed to be going on.
Though it also helps if they actually care. That was a bit of a problem with the official meeting as well, the code had remarkably well defined territories within it. But that's a culture problem.I agree here heavily. I don't think code reviews necessarily need a formal meeting attached. At least that's been my positive experience, an informal gathering of 2 or 3 people at one's desk, after all have read and understood the code / project / requirements, seems to be effective from what I remember. The part regarding, if they actually care is very important, too. That's why I don't recommend too senior of people reviewing too low level code, as well as peers that are distantly positioned on a project. People are busy, I can understand why sometimes the code isn't looked at too deeply.
"I have a theory that the truth is never told during the nine-to-five hours. " — Hunter S. Thompson
-
A pig walked into a bar with a tank of liquid nitrogen and forty-two CListCtrls... Hmm... Nah, doesn't work.
Proud to have finally moved to the A-Ark. Which one are you in?
Author of the Guardians Saga (Sci-Fi/Fantasy novels)Very creative, you should be a writer!
Help a brotha out and vote Managing Your JavaScript Library in ASP.NET as the best ASP.NET article of May 2011.
-
I have a new team forming up and we want to decide on the best way to formalize code reviews. Is the Code Review the responsibility of one person or the whole team? Is each, or certain, team member(s) allowed to review others' code or is there only one person who does the reviews? If it's only one person, then who reviews his/her code? I've heard of both in practice and some of the teams where I'm at use both patterns. I'm just looking for insight as to what everyone thinks is the best way to go and why.
- Arcond
I think it should be responsibility of one person/team dedicated for the task - for better results - if you need to ship your code ASAP you are not going to sort out the mess in other's even if boss says so, on being pressed you will complete the formality and move on. Others can point out the issue if they found any anonymously - until unless direct intervention is required-, this will make team go smooth - my code is my pride-. Edit--- Looks like there is some issue with signature rendering so removed it from the post.
-
I have a new team forming up and we want to decide on the best way to formalize code reviews. Is the Code Review the responsibility of one person or the whole team? Is each, or certain, team member(s) allowed to review others' code or is there only one person who does the reviews? If it's only one person, then who reviews his/her code? I've heard of both in practice and some of the teams where I'm at use both patterns. I'm just looking for insight as to what everyone thinks is the best way to go and why.
- Arcond
Usually, you review each other's code. If your application processes highly sensitive data, it might be a good idea to hire consultants that specialize in code security.
-- Kein Mitleid Für Die Mehrheit
-
I think it should be responsibility of one person/team dedicated for the task - for better results - if you need to ship your code ASAP you are not going to sort out the mess in other's even if boss says so, on being pressed you will complete the formality and move on. Others can point out the issue if they found any anonymously - until unless direct intervention is required-, this will make team go smooth - my code is my pride-. Edit--- Looks like there is some issue with signature rendering so removed it from the post.
Amar Chaudhary wrote:
you are not going to sort out the mess in other's even if boss says so, on being pressed you will complete the formality and move on.
Are you saying that if you were aware of a flaw in a colleagues code, you would not mention it?
Henry Minute Do not read medical books! You could die of a misprint. - Mark Twain Girl: (staring) "Why do you need an icy cucumber?" “I want to report a fraud. The government is lying to us all.” I wouldn't let CG touch my Abacus! When you're wrestling a gorilla, you don't stop when you're tired, you stop when the gorilla is.
-
I have a new team forming up and we want to decide on the best way to formalize code reviews. Is the Code Review the responsibility of one person or the whole team? Is each, or certain, team member(s) allowed to review others' code or is there only one person who does the reviews? If it's only one person, then who reviews his/her code? I've heard of both in practice and some of the teams where I'm at use both patterns. I'm just looking for insight as to what everyone thinks is the best way to go and why.
- Arcond
IMHO everyone should review everyone's code; it is more in the spirit of the team - when reviewing someone's code it isn't a 'bad' thing to find issues, it's a positive thing. less experienced programmers get to see other's code more frequently than otherwise, and having to review it makes them take the time to understand and learn more experienced programmers are sometimes a little, well, set in their ways. Having others review their code is sometimes a learning experience for them, too any developer may look at someone's code and ask themselves "I wonder why they did it this way rather than that way?" If the question is asked, at least one of the participating parties learns something standards are a fluid thing - so multiple code reviewers allow discussions (aka raging flame-wars) to take place regarding standards and, hopefully, consensus can be reached Having one person do the reviews is prone to problems - the person leaves/is off sick/ is a loony/ with one person doing reviews, who reviews their code?
MVVM# - See how I did MVVM my way ___________________________________________ Man, you're a god. - walterhevedeich 26/05/2011 .\\axxx (That's an 'M')