in reply to Code review

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.

Replies are listed 'Best First'.
Re: Re: Code review
by jlawrenc (Scribe) on Mar 26, 2001 at 20:43 UTC
    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]
Re: Re: Code review
by husker (Chaplain) on Mar 26, 2001 at 21:13 UTC
    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]
        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
[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]
        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.

[reply]

In Section Perl Monks Discussion