[Edinburgh-pm] CPAN upload

[Aaron Crane]

Miles Gould <miles at assyrian.org.uk> wrote:
> It looks like everything went OK, because this is now the first hit
> for List::Priority on search.cpan.org:
>
> http://search.cpan.org/~mgould/List-Priority-0.03rc1/Priority.pm

It's also on Metacpan, although you get the 0.02 version by default:

https://metacpan.org/release/MGOULD/List-Priority-0.03rc1

I think that's the expected result for a dev release (and it looks
like *something* is treating "0.03rc1" as a dev-release version).

> Accordingly, I've closed the "shift does the same thing as pop!" bug
> that led me to fork the module in the first place.

Huzzah!

> v0.03
> - basically, what's there now: the code now does what the
> documentation says it does, which is all that's really required in a
> module which has apparently been sitting around largely unloved for
> nine years.
> - rewrite some of the docs to (a) be less ugly, (b) be more
> grammatical, (c) make the useful options{SIZE} feature more apparent.
> - MOAR TESTS

If you wanted to do any whitespace cleanups (convert from DOS format
to Unix, delete line-final spaces, adjust indentation, etc), this
would probably be a reasonable place to do them.

I suggest also looking at Makefile.PL.  The current version claims
that the author is one A. U. Thor <a.u.thor at a.galaxy.far.away>, which
seems moderately unlikely.  It also fails to declare your new code's
implicit test-time dependency on Test::More 0.88 (which is the first
version to provide done_testing()).  The simple way to do that is to
add it to PREREQ_PM:

    WriteMakefile(
        …
        PREREQ_PM => {
            'Test::More' => 0.88,
        },
        …
    );

> v0.04
> - replace the current horrible "return an error string when given the
> wrong number of arguments" design with calls to croak(). (I'm thinking
> an API-breaking change deserves to be separated from a bugfix
> release).
> - give some guidance in the docs as to what, if any, reason there is
> to use this module over the other eleventy bajillion priority queues
> on CPAN.

Both of these sound very useful.

> 1. Does this sound sane?

Yes, with one caveat.  Given the existence of 0.03rc1, I'm not
completely sure whether 0.03 would be considered older or newer than
it.  One could in theory find out, but if I were in your situation,
I'd probably just call my next release 0.04 and be done with it —
rather easier.

Also, the "return an error string" thing strikes me as something that
noone could possibly be relying on, and it's clearly not a good
design.  If you chose to do a single release with both sets of changes
you describe, I don't think that would be unreasonable.

> 2. There's the potential for confusion between the recently-added
> size() method (number of elements currently in the list) and
> options{SIZE} (*maximum* number of elements the list will hold).
> Should I rename one? What to?

Hmm, that's annoying — I think the natural names are `size` for the
new method, and `capacity` for the constructor option; but adopting
that convention means that the thing to be renamed is the one that's
been around longest.  One possible approach to dealing with that is to
document a `capacity` option, and use that name in the `insert`
method, but support a `SIZE` option on construction so old callers
don't break.  Concretely, the constructor would look something like
this:

    sub new {
        my ($class, %options) = @_;
        $options{capacity} = delete $options{SIZE}
            if exists $options{SIZE} && !exists $options{capacity};
        return bless { size => 0, options => \%options }, $class;
    }

> 3. What would you suggest doing in the other cases where functions
> return error strings, namely "you tried to add an element to a list
> that's already full of higher-priority things" and "you tried to add
> something that's already in the list, and we don't support that for
> reasons best known to the original author". croak()ing seems excessive
> in this situation, but returning error strings doesn't seem very
> sensible either.

I'd say the options are:

- remove the limitations that cause those to be error conditions,
where reasonable
- use croak(); this doesn't add a dependency because Carp has been in
core since 5.000
- use die instead of croak()

For me, that's the order of preference (best first).

-- 
Aaron Crane ** http://aaroncrane.co.uk/