Slash Boxes
NOTE: use Perl; is on undef hiatus. You can read content, but you can't post it. More info will be forthcoming forthcomingly.

All the Perl that's Practical to Extract and Report

use Perl Log In

Log In

[ Create a new account ]

rjbs (4671)

  (email not shown publicly)
AOL IM: RicardoJBSignes (Add Buddy, Send Message)
Yahoo! ID: RicardoSignes (Add User, Send Message)

I'm a Perl coder living in Bethlehem, PA and working Philadelphia. I'm a philosopher and theologan by training, but I was shocked to learn upon my graduation that these skills don't have many associated careers. Now I write code.

Journal of rjbs (4671)

Friday December 01, 2006
10:05 PM

reviews are not bug reports

[ #31783 ]

Sometimes, I see a bad review of a module on the CPAN, and I wonder why it wasn't filed as a bug report instead. For example, I recently saw this review of the popular and indispensable Time::Local. The reviewer makes three points:

  1. The module thinks that there are 30 days in October.
  2. The module stops executing (dies?) when an invalid date is given.
  3. The module doesn't parse the date for you.

Well, the module doesn't think there are 30 days in October:

DB<1> p "@{[localtime(timelocal(0,0,0,31,9,106))]}"
0 0 0 31 9 106 2 303 0

Probably the reviewer didn't pay attention to the fact that months run from zero to eleven, not from one to twelve, and he tried this instead:

DB<2> p "@{[localtime(timelocal(0,0,0,31,10,106))]}"
Day '31' out of range 1..30 at (eval 22)[/usr/local/lib/perl5/5.8.8/] line 2

The behavior makes sense, since the module provides, as documented, "the inverse of build-in perl functions localtime() and gmtime()." If this had been filed as a bug report, the reviewer could have been set straight.

The modules does, as we see above, throw an exception if the input is invalid. That's a documented feature, though, and easy to deal with by using eval. If a bug report had been filed, the reviewer could have been set straight.

As for the third complaint, it's true. The module does not parse dates, because it is not for parsing dates. The reviewer suggests other modules which do parse dates, but don't perform the functions that Time::Local performs. I wonder if we will shortly see a negative review of Time::ParseDate for its inability to convert from a time struct to epoch seconds.

I see a lot of these reviews, which are clearly written by people who don't know what they're talking about. They amuse (or irritate) people who have a clue, and they just confuse or misdirect people who might otherwise have used the right tool for the right job.

I will write no more negative reviews of a module until I have filed bug reports that are either ignored or rejected, unless the problems are clearly major design issues. I urge all other reviewers to do the same.

The Fine Print: The following comments are owned by whoever posted them. We are not responsible for them in any way.
More | Login | Reply
Loading... please wait.
  • Bravo, nicely said. It's like moderation advice on Slashdot: don't waste your effort moderating the volumnious bad content. Instead, focus your efforts on rewarding the good efforts.

    Virtuous cycles are better than vicious cycles.

    Side note: I've noticed that moderation is very rarely used on Instead, people seem to use the "friend" feature to acknowledge good posts. I think that's just great. It's like permanently modding someone up.
    • I do think it's worth noting that I'm not entirely against negative reviews -- but they should address things like, "critical bugs have been ignored for months/years" or "this module is broken as designed, because it can never handle years after 1978." Another problem is that if bugs ARE eventually fixed, or years after 1978 are omitted on purpose, the author has no way to reply to the reviewer to ask him to reconsider, as reviews are effectively anonymous: there is a name, but no means to contact.
  • I agree. Thank you for expressing this so clearly.