|
Re: Code review
by frankus (Priest) on Mar 26, 2001 at 16:12 UTC
|
I think code review is a good thing, especially on Perl Monks
there is a great potential for good.
The problem with peer code review in real life is that to
start with there are often enormous personality clashes and
then everyone might get on, of course there's a slight
disturbance everytime a new person joins; therein lies
the rub: there'll be new folk joining and opining constantly
. How do you stop people having flame wars on the placing of
brackets, or something equally irrelevant?
If this excellent suggestion is to be used and be fruitful,
there will need to be some rules on what can be commented
upon.. i.e Larry Wall's formatting rules should be applied,
other than that I've run out of suggestions, but for what
it's worth I really like this idea.
--
Brother Frankus. | [reply] |
|
|
I agree that code review has merrit. I could toss in a
few spins that might be interesting as well.
Review criteria
There might be a skeleton of criteria to examine code by.
- formatting and "standards" compliance
- error handling
- clarity
- maintainability
- etc.
Obviously this just to mention some of the domains where
by code should be reviewed.
Eligibility
I think that other areas serve the new and developing programmers well.
Personally, I don't necessarily want to see reviews of
yet another input form processing program.
Perhaps candidates need to accomplish *something* before
being sumitted to the review queue.
Review board
Same sort of idea goes for reviewers. If someone is going
to review code they need to provide constructive feedback
and be skilled at not totally shredding someone else's
code.
On a personal note. I have a module that I'd like to share
but am anxious to do so. I have fears of total shreddage
if I do so. For me, I'd like to have at least one round
of private code review at which point I'll feel more comfortable
with public criticisms. In my case, I'm working with
someone like myself from my local Perl
Mongors group.
Perhaps we could have a 1:1 mentoring mechanism with the
purpose of facillitating that process.
| [reply] |
|
|
Perhaps one of the ground rules is that "review" should not address "style", which is usually quite subjective. Rather, "review" should encompass issues of a) correctness b) robustness c) elegance d) performance e) portability f) re-usability g) security h) applicability. And maybe some other stuff I can't think of right now.
As for who should be able to review, perhaps code review should be limited to monks who have attained a certain XP level (although that opens a whole new vista on the whole "XP whoring" issue), or perhaps a panel appointed (at first) by Vroom and then later the board can add members to itself as it sees fit.
Hopefully, having the correct people on the panel, combined with a good , would limit flmaing and encourage useful discourse.
| [reply] |
|
|
I agree with all of this, but having the review open to
people above a certain XP is problematic, it would make the
code not entirely peer review. Which in industry I've found
a bad thing, it lacks democracy for one thing. I know lots
of really good developers with lower XP than me.
But then what do you do? As a first pass, this seems to be
a compromise but a good starting place1. IMHO FWIW
--
Brother Frankus.
1. Of course I'm perhaps being presumptuous to assume a Friar would get this priviledge ;-) | [reply] |
|
|
|
|
I disagree. Style often has objective effects on code
quality. Style often has no effect other than the need to
be consistent. Quality review should comment on style but
note which decisions fall into which class.
For instance any indent from 2-4 is effective. So are
various placements of the brace. However 6-8 space
indents reduce comprehension.
For instance verbose variable names and abbreviated ones
are not (AFAIK) significantly different. But flags whose
name does not indicate what true means, and variables
with meaningless names like $x significantly reduce
comprehension.
I could go on. But the point should be clear. Without a
coding standard to judge by, the
various arbitrary decisions that must be made should not
be advocated. But there are real style issues, and I
think it is valuable to talk about those.
| [reply] |
|
|
Re: Code review
by little (Curate) on Mar 26, 2001 at 16:55 UTC
|
| [reply] |
|
|
--
Brother Frankus. | [reply] |
|
|
| [reply] |
|
|
" So what happened last time to make this erm not happen?"
dunno
but I guess it was forgotten. Actually I think Tim Vroom is very busy with is exams and stuff, so he's got no time. As I understand the everything engine, it'S very simple to add a new section, but harder to mainatin it. So actually I think we should simply have the next polling asking for:
"What about a code review section?"
- We need that
- might be nice
- don't bother
- blah ...
Or Tim might think about letting have some of the Saints in our Book acces to site but not mainly the engine to work and to develope the Site, because the most content and structure of the site resides in XML files and structure definitions outside the code.
Have a nice day
All decision is left to your taste
| [reply] |
