On Monday 25 December 2006 21:44, James Keenan via RT wrote:

A few style comments here.

> Utils.pm
>     unshift @{$allargsref->{include}}, (
>         ".",
>         "$FindBin::Bin/../..",
>         "$FindBin::Bin/../../src/pmc/"
>     );

Why no File::Spec here?


>     if (File::Spec->file_name_is_absolute($file) && -e $file) {
>         return $file;
>     }

There's File::Spec here.

> =head3 C<dump_vtable()>
>
>     $self->dump_vtable("$FindBin::Bin/../../vtable.tbl");

... but not here (this is documentation).

> sub dump_vtable {
>     my $self    = shift;
>     my $file    = shift;

Why two shifts here, when @_ goes unused through the rest of the method?

> sub print_tree {
>     my $self  = shift;
>     my $argsref = shift;

Ditto here.

> sub find_and_parse_pmc {
>     my ($self, $file) = @_;

... but not here.

> sub gen_parent_list {
>     my $self = shift;
>     my ($name, $all) = @_;

This one just confuses me.

-- c

Reply via email to