Praha.pm logo

The Pull Request Club

A lightening talk

E. Choroba

Praha.pm (Prague)

Currently unemployed

PerlMonks, CPAN, StackOverflow

choroba@matfyz.cz

How does it work?

  1. You register at https://pullrequest.club.
  2. You’re assigned a GitHub repository (usually corresponding to a CPAN distribution).
  3. You submit a pull request to the repo.

Why?

Organised by Kıvanç Yazan.

Thank you 🙏

Calendar::Simple

META.json at metacpan.org

This was missing in the file:

   "resources" : {
      "bugtracker" : {
         "web" : "https://github.com/davorg/calendar-simple/issues"
      },
      "license" : [
         "http://dev.perl.org/licenses/"
      ],
      "repository" : {
         "type" : "git",
         "url" : "https://github.com/davorg/calendar-simple.git",
         "web" : "https://github.com/davorg/calendar-simple"
      }

META.json at metacpan.org

The file created by

perl Build.PL && ./Build dist

contained the missing lines.

Solved

I let Dave Cross know. He reran the command and uploaded the fixed tarball to CPAN.

🙁 But no pull request to close my assignment.

Reading the documentation of the pcal command…

As of version 2.0.0, the default start_day has changed. Previously, it was Sunday; now, it is Monday. This is so the default behaviour matches that of the standard Unix cal command.

But

Reading the documentation…

As of version 2.0.0, the default start_day has changed. Previously, it was Sunday; now, it is Monday. This is so the default behaviour matches that of the standard Unix cal command.

In fact, the behaviour depends on the locale (LC_TIME).

A pull request!

I added an option to specify the week should start on Sunday (already supported by the library).

App::perlimports

The failing code

    my $doc = App::perlimports::Document->new(
        filename => $filename,
        logger   => $logger,
        $source_text ? ( selection => $source_text ) : (),
    );

    return App::perlimports::Include->new(
        document => $doc,
        include  => $doc->includes->[0],
        logger   => $logger,
        %{$pi_args},
    );

Where are the includes coming from?

App::perlimports::Document

has includes => (
    is          => 'ro',
    isa         => ArrayRef [Object],
    handles_via => 'Array',
    handles     => {
        all_includes => 'elements',
    },
    lazy    => 1,
    builder => '_build_includes',
);

The builder

App::perlimports::Document

sub _build_includes {
    my $self = shift;

    return $self->_ppi_selection->find(
        sub {
            $_[1]->isa('PPI::Statement::Include')
                && !$_[1]->pragma     # no pragmas
                && !$_[1]->version    # Perl version requirement
                && $_[1]->type
                && ( $_[1]->type eq 'use'
                || $_[1]->type eq 'require' )
                && !$self->_is_ignored( $_[1] )
                && !$self->_has_import_switches( $_[1]->module )
                && !App::perlimports::Sandbox::eval_pkg(
                $_[1]->module,
                "$_[1]"
                );
        }
    ) || [];
}

Sandbox

sub eval_pkg {
    my $module_name = shift;
    my $content     = shift;

    my $pkg = pkg_for($module_name);

    my $to_eval = <<"EOF";
package $pkg;
$content;
1;
EOF

    local $@;
    eval $to_eval;

    my $e = $@;
    return $e;
}

The exception

LWP::UserAgent version 6.49 required--this is only version 6.31 at (eval 401) line 2.

The test

use Test::Needs {
    'Cpanel::JSON::XS' => 4.19,
    'Getopt::Long'     => 2.40,
    'LWP::UserAgent'   => 5.00,
    'Test::Script'     => 1.27,
};

...

subtest 'with version' => sub {
    my $pi = source2pi(
        'test-data/with-version.pl',
        'use LWP::UserAgent 6.49;',
    );

    ok( !$pi->_is_ignored, '_is_ignored' );
    is(
        $pi->formatted_ppi_statement,
        'use LWP::UserAgent 6.49 ();',
        'formatted_ppi_statement'
    );

The pull request

I lowered the expected version to fit into the test requirements.

Thank you

https://e-choroba.eu/23-prclub