New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attempt to fix Issue #12. #15

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@monsieurp
Contributor

monsieurp commented Dec 15, 2015

The code speaks for itself, I have documented every new line of code.

It probably needs more love. @zoffixznet let's discuss it on IRC.

This is an attempt to fix Issue #12.
The code speaks for itself, I have documented every new line of code.

It probably needs more attention and love.
@zoffixznet

This comment has been minimized.

Collaborator

zoffixznet commented Dec 15, 2015

Hi,

Thanks for the PR, but I see numerous issues with it.

  1. There appears to be some debugging code (Data::Dumper) left over here, here, and here
  2. Even if I remove that code, the tests still fail. If you run prove -vlr t/ you can see the errors yourself.
  3. I'm not completely sure of the purpose of the test.pl file. Unmodified, it does not pass tests (try running it with prove -v test.pl), but even if it did, it should be located in the t/ directory, along with all the other tests and have a .t extension for consistency.
  4. Lastly, there appears to have been some confusion with regard to what the problem described in Issue #12 is. First, in your test file, I see you have two =head1 SYNOPSIS POD sections, which I doubt people regularly do in their POD. Issue #12 describes a single SYNOPSIS section with multiple code segments (example below). And the interference mentioned in the Issue doesn't seem to be tested for in your test file, as the two code chunks have nothing to do with each other. Here's the example POD that should passs the test without trouble if the Issue is resolved. Currently, it'll fail, because the two code chunks interfere with each other and the second one generates the "my" variable $x masks earlier declaration in same scope warning:
=pod

=head1 SYNOPSIS

=for test_synopsis use warnings FATAL => 'all';

First sample code:

    my $x = 42;

Second sample code:

    my $x = 43;

=head1 DESCRIPTION

bleh

=cut

Please see if you can address these issues.

@monsieurp

This comment has been minimized.

Contributor

monsieurp commented Dec 16, 2015

I will address these issues in a separate PR. Thanks!

@monsieurp monsieurp closed this Dec 16, 2015

@zoffixznet

This comment has been minimized.

Collaborator

zoffixznet commented Jan 4, 2016

The issue is now fixed with d26775f

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment