Pull requests

#16 Merged
Repository
tobyink/p5-xml-libxml p5-xml-libxml
Branch
default
Repository
shlomif/perl-XML-LibXML perl-XML-LibXML
Branch
default

XML::LibXML::Element hash deref - take 4

Author
  1. Toby Inkster
Reviewers
Description

All the rationale is here... https://bitbucket.org/shlomif/perl-xml-libxml/pull-request/15/new-improved-and-leak-free-hash-deref-for

UPDATE: I have addressed your comments now.

  • Learn about pull requests

Comments (4)

  1. Shlomi Fish repo owner

    Hi Toby,

    sorry that it took me so long to review this pull request (job and some other projects intervened). First of all I should note that this pull request changes a lot of the formatting of code that does not need to be changed. Since it may be in separate commits, it would be OK.

    Otherwise, I don't like the fact that in the new *.t files there are:

    1. Statements at the end of blocks without a trailing semicolon. All statements should end with a semicolon (see Perl Best Practices and other sources).

    2. The arguments to the testing assertion function calls are not enclosed in parentheses. They should be.

    I'll add these things to HACKING.txt.

    Regards,

    -- Shlomi Fish

      1. Shlomi Fish repo owner

        Hello Toby!

        Sorry it took me so long, but your changes break XML::LibXML here:

        PERL_DL_NONLAZY=1 /usr/bin/perl5.14.2 "-MExtUtils::Command::MM" "-e" "test_harness(0, 'blib/lib', 'blib/arch')" t/*.t
        syntax error at /home/shlomif/progs/perl/cpan/XML/LibXML/hg/pull-req/blib/lib/XML/LibXML.pm line 1512, near "require Hash::FieldHash 0.09"
        BEGIN not safe after errors--compilation aborted at /home/shlomif/progs/perl/cpan/XML/LibXML/hg/pull-req/blib/lib/XML/LibXML.pm line 1521.
        Compilation failed in require at t/01basic.t line 6.
        BEGIN failed--compilation aborted at t/01basic.t line 6.
        # Looks like your test exited with 255 before it could output anything.
        t/01basic.t ......................... 
        Dubious, test returned 255 (wstat 65280, 0xff00)
        Failed 3/3 subtests 
        syntax error at /home/shlomif/progs/perl/cpan/XML/LibXML/hg/pull-req/blib/lib/XML/LibXML.pm line 1512, near "require Hash::FieldHash 0.09"
        BEGIN not safe after errors--compilation aborted at /home/shlomif/progs/perl/cpan/XML/LibXML/hg/pull-req/blib/lib/XML/LibXML.pm line 1521.
        Compilation failed in require at /home/shlomif/progs/perl/cpan/XML/LibXML/hg/pull-req/blib/lib/XML/LibXML/Common.pm line 28.
        BEGIN failed--compilation aborted at /home/shlomif/progs/perl/cpan/XML/LibXML/hg/pull-req/blib/lib/XML/LibXML/Common.pm line 28.
        Compilation failed in require at t/02parse.t line 13.
        BEGIN failed--compilation aborted at t/02parse.t line 13.
        # Looks like your test exited with 255 before it could output anything.
        t/02parse.t ......................... 
        Dubious, test returned 255 (wstat 65280, 0xff00)
        Failed 531/531 subtests 
        syntax error at /home/shlomif/progs/perl/cpan/XML/LibXML/hg/pull-req/blib/lib/XML/LibXML.pm line 1512, near "require Hash::FieldHash 0.09"
        

        Can you look into it? I'm using perl-5.14.2-5.mga2 on Mageia Linux 2/Cauldron. I'm also getting it with:

        /home/shlomif/apps/perl/perlbrew/perls/perl-5.14.2-64bitall-longdouble/bin/perl

        Regards,

        -- Shlomi Fish