Re: Re: Code review

[husker]

in reply to Re: Code review
in thread Code review

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.

[frankus]

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 place¹. IMHO FWIW

--

Brother Frankus.

1. Of course I'm perhaps being presumptuous to assume a Friar would get this priviledge ;-)

[little]

There was also long ago a thread discussing the question if the "perl guru's" like Wall, Schwartz, Christiansen (forgive me my funny and short list, it's not meant as anything else than a short list) should get tons of XP in advance, but as far as I remember all agreed that everyone has to earn his XP. And very professional perl coders might say welcome to those who ask them in any other way (just as merlyn does by stating that he welcomes questions about questionable code by e-mail). But I would recommend to NOT TO LET MONKS decide whether a monk is allowed to access this section or not. Just keep it simple.

Have a nice day
All decision is left to your taste

[tilly]

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.

[husker]

I agree that some decisions that we would consider "style" decisions do impact maintainability, and good maintainability depends on clarity and "comprehendability" of code. So in that sense, some critique of style is appropriate with regards to maintainability. So I would really consider these "maintainability" issues, and not "style" issues per se. Anything which is purely a "style" issue should be passed over.

Other areas which affect maintainability would be documentation, generalization/modularity, and abstraction.