We don't do code reviews..
-
@JoeCool said:
I once worked with a guy who got filthier as the week went by, but was clean every Monday. I'm guessing he showered every Sunday night whether he needed it or not.
He did need it. No doubt about that.
-
@Zylon said:
I think it's the nice thing to do... I'm sure it's the only thing Nagesh has eaten all week.@PJH said:
@Nagesh said:
But # 1 reason for failure of project is client not ready to sing off on on deliveries that is previously agreed up on by him..
What if they're terrible at karaoke?Don't feed the troll.
-
@C-Octothorpe said:
@Zylon said:
I think it's the nice thing to do... I'm sure it's the only thing Nagesh has eaten all week.@PJH said:
@Nagesh said:
But # 1 reason for failure of project is client not ready to sing off on on deliveries that is previously agreed up on by him..
What if they're terrible at karaoke?Don't feed the troll.
Which is why you shouldn't do it. Like a stray cat, if we stop feeding him he'll go away. Or starve to death. Either is fine.
-
@TheRider said:
@JoeCool said:
I once worked with a guy who got filthier as the week went by, but was clean every Monday. I'm guessing he showered every Sunday night whether he needed it or not.
He did need it. No doubt about that.
-
The Zoidberg version is better, because it has Zoidberg.
-
-
No, the "Hey Ringo" one:
I need to break out the Paint.NET and add my Impact captions to it though.
-
My policy: we don't do code reviews because they're not productive in my experience. We end up talking about variable naming at worst or pet design patterns at best, until suddenly it's lunchtime and we've accomplished nothing. Make your code work or get fired.
-
-
@bridget99 said:
We end up talking about variable naming at worst or pet design patterns at best, until suddenly it's lunchtime and we've accomplished nothing.
That doesn't sound like a code review, it sounds more like a discussion about good and bad practise - which I'd have expected to have already taken place during policy formulation for organisational coding standards... so that code reviews would stay on focus.
Either way, discussions of that nature still have value: although you may feel nothing is accomplished, inexperienced coders will benefit greatly from experienced mentoring. But there is a time and a place, after all.
-
@bridget99 said:
My policy: we don't do code reviews because they're not productive in my experience. We end up talking about variable naming at worst or pet design patterns at best, until suddenly it's lunchtime and we've accomplished nothing.
It's best to do the reviewing offline, perhaps preceded by the developer giving a brief, high level talk about what he's trying to accomplish, the approach he chose, etc., if this info would be a good entry into the code. After the review, you get together to discuss problem areas or bugs.
When the reviewers meet with the coder, someone has to be the moderator, to keep discussions on track and to hand out action items.
At my last gig we had a software tool that would let you view the code, post comments, and flag problem areas; wish I could recall the name, but it worked well.
-
@rstinejr said:
@bridget99 said:
My policy: we don't do code reviews because they're not productive in my experience. We end up talking about variable naming at worst or pet design patterns at best, until suddenly it's lunchtime and we've accomplished nothing.
It's best to do the reviewing offline, perhaps preceded by the developer giving a brief, high level talk about what he's trying to accomplish, the approach he chose, etc., if this info would be a good entry into the code. After the review, you get together to discuss problem areas or bugs.
When the reviewers meet with the coder, someone has to be the moderator, to keep discussions on track and to hand out action items.
At my last gig we had a software tool that would let you view the code, post comments, and flag problem areas; wish I could recall the name, but it worked well.
All great suggestions [ and if you are using Visual Studio, those capabilities are all available]. I find that having pre/post code reviews to be the most helpful. Having a discussion about the details before the actual coding can save significant time. Often there are cases where it is "too late" when the code is written to make a change (in a cost effective manner), that would have been a great idea if it had been done that way in the beginning.
-
@Cassidy said:
Either way, discussions of that nature still have value: although you may feel nothing is accomplished, inexperienced coders will benefit greatly from experienced mentoring. But there is a time and a place, after all.
Mentoring by Bridget99? Hah! Oh wait, you're serious? Let me laugh even harder.
-
@rstinejr said:
At my last gig we had a software tool that would let you view the code, post comments, and flag problem areas; wish I could recall the name, but it worked well.
Crucible does that.
-
@bridget99 said:
My policy: we don't do code reviews because they're not productive in my experience. We end up talking about variable naming at worst or pet design patterns at best, until suddenly it's lunchtime and we've accomplished nothing. Make your code work or get fired.
Those are non-functional specifications. They should be written down and agreed upon, and thenceforth stuck to. If the need arises, you request a change to the non-functional specifications because they don't work, but you don't deviate from them because you disagree with them. If that happens, the manager has to deal with it (a private word, a stern warning, sacking, whatever it takes).Code review should be about sticking to the non-functional specifications (which is a simple yes/no) and the WTFs that you can't cover with a list of non-functional specifications. And the person doing the code review should be experienced enough, and have the clout and/or authority to enforce this.
We have plenty of code at work that works. But if you have to make a change, you're lost in a jungle of code, and often rewriting it is a viable alternative. Or it would work until a certain parameter changes (for example, it turned out not to be thread-safe after all), or an external factor changes.
Code that works is simply not enough. It has to work, and be understandable by other developers, and given a clean bill by static source code analysis, and comply with the non-functional specifications from both the development section and the support people.
-
@morbiuswilters said:
@rstinejr said:
At my last gig we had a software tool that would let you view the code, post comments, and flag problem areas; wish I could recall the name, but it worked well.
Crucible does that.
That was it! I could not for the life of me remember the name of the damn thing.
I didn't mention this in my post, but Crucible integrates with your source control, so it's really easy to focus on the diffs.
I like it; I think shops can use this tool to improve the quality of their software.
-
@rstinejr said:
@morbiuswilters said:
@rstinejr said:
At my last gig we had a software tool that would let you view the code, post comments, and flag problem areas; wish I could recall the name, but it worked well.
Crucible does that.
That was it! I could not for the life of me remember the name of the damn thing.
I didn't mention this in my post, but Crucible integrates with your source control, so it's really easy to focus on the diffs.
I like it; I think shops can use this tool to improve the quality of their software.
The last time I used it (a few years ago) it was very good. But Atlassian has a habit of taking a good UI and running it into the ground, so I don't know if that's still the case.
-
