Code Reviews.
-
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[^] -
How do you guys do them?
cheers, Chris Maunder The Code Project | Co-founder Microsoft C++ MVP
I give my coworkers code a quick glance over when I pull it from source control and send an email if something looks odd. AFAIK I'm the only person who does this. Everyone always agrees when the subject is raised that it's a 'good idea' and if there was 'more money in the budget/time in the schedule' we should start doing them. 'Maybe on the next funding increment.' Which is why I do what I do. :sigh: :sigh:
Did you ever see history portrayed as an old man with a wise brow and pulseless heart, waging all things in the balance of reason? Is not rather the genius of history like an eternal, imploring maiden, full of fire, with a burning heart and flaming soul, humanly warm and humanly beautiful? --Zachris Topelius Training a telescope on one’s own belly button will only reveal lint. You like that? You go right on staring at it. I prefer looking at galaxies. -- Sarah Hoyt
-
How do you guys do them?
cheers, Chris Maunder The Code Project | Co-founder Microsoft C++ MVP
We have developed an in-house tool that keeps track of all the changes to the source code. Then all developers have a mandatory review day, once every two weeks (not on the same day). For each review, there is a check list of things to go through. We are encouraged to review code for an area that we are at least a bit familiar with so that we can better assess the impact of those changes on the rest of the functionality. The review results are submitted via the same tool and the developer can make the suggested changes or reply with an explanation of why those changes are not necessary/helpful/whatever. One of our biggest problems, though, is that the reviewers differ widely in opinion sometimes and sometimes some of them can be quite pedantic.
-
How do you guys do them?
cheers, Chris Maunder The Code Project | Co-founder Microsoft C++ MVP
We use ReviewBoard to post code reviews from git diffs. The RB link is then sent out to relevant people in the group for review and sign-off before pushing the code to 'central' repository. When the diff is being 'posted' to RB, the script that does the job amends the commit message to include the RB link to which the diff was sent for review and hence each commit pushed would ideally have an RB link where you can see who approved the change.
a.k.a. Vite Phoenix and Vite Zeus... Proud member and co-founder of OlympianZ
-
How do you guys do them?
cheers, Chris Maunder The Code Project | Co-founder Microsoft C++ MVP
Well actually, we do it ourselves in peer review sessions with lots of manic laughters, booze, mantras and faith.
-
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
You are always right. If everyone used the force then they would be right as well. I often close my eyes when coding. The force does work Nagy Vilmos. Just wish that the code reviewers and the network pigs would do the same.....
"Rock journalism is people who can't write interviewing people who can't talk for people who can't read." Frank Zappa 1980
-
How do you guys do them?
cheers, Chris Maunder The Code Project | Co-founder Microsoft C++ MVP
One Cardinal Rule: The code doesn't change unless you can convince the author that the change is necessary or useful. This really sets the tone for the code review, it prevents "religious wars" and makes sure that the review is all about understanding the reason why someone chose to do things this way.
-
How do you guys do them?
cheers, Chris Maunder The Code Project | Co-founder Microsoft C++ MVP
All of the software my department writes is in-house apps used only be our lines of business, or web apps used by our clients. Nothing we do is shipped. We use Visual Studio's static code analyis for the simple stuff. We have some items that are globally suppressed (such as null parameter checking), and temporary local suppressions for other items. With exceptions only for platform ports, temporary suppressions are actually temporary; we GREP them out of the code before it gets to any end user testing. With a platform port (which we're pushing now because some of our stuff, even though working, is *very* old, and we can't get support for the tech stack), especially those that are coming from "legacy" code, i.e. with no automated tests, the algorithms are retained until such time as tests can be wrapped around them to verify functionality. Even those seldom go to production, though, and when they do, the very next iteration for that app is likely to get them stripped out. We use NCover, and lately OpenCover, to automate the test coverage. We're aiming for 80% right now, and are at near 90% for the greenfield and completely ported stuff, though overall, because of the "legacy" code, we're still in the 40% range. We've been using the Redgate stuff for performance monitoring and memory management.
Currently reading: "Gateway", by Frederik Pohl
