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

Pod corrections + 2 more test files for more explicit direct sub tests #4

Merged
merged 2 commits into from Feb 22, 2018

Conversation

Projects
None yet
2 participants
@ThornyS

ThornyS commented Feb 22, 2018

Hi Ron,
I've recently signed up to help contribute to CPAN via the PRC i.e. 'P'ull 'R'equest 'C'hallenge.
I was assigned the GraphViz2 distribution (well, after the first one scaring the bejeezers out of me 'Math::Prime::Util' I asked for a re-assignment) and directed to some helpful ideas on how to make a useful pull request.

I've done the following to generate this current pull request

  1. find all .pl and .pm files and run podchecker against them. e.g.
    cd /path/to/GraphViz2
    find ./ -name "*.p[lm]" -exec podchecker {} \;
    I found 2 errors, a typo and a head hierarchy clash.
  2. This one I am more dubious about as I'm not massively familiar with Devel::Cover in fact this was my first outing with this package. So I ran the following below and looked at the generated ./cover_db/coverage.html e.g.
    cd /path/to/GraphViz2
    cover -test
    I'm in doubt as to whether GraphViz2 requires direct sub checking as the scripts/ tests usable examples rather than single function calls. However I've created 2 more specific test files that do just this, so the final coverage.html shows a much greater percentage covering of subs. NB without fully understanding what the tests are doing I've added minimal arguments to these subs so the sub called executes without error. The new test files are below and will run with make test or cover -test
    t/test_new.t
    t/test_more_methods.t

Please let me know what you think

@ThornyS

This comment has been minimized.

Owner

ThornyS commented on lib/GraphViz2.pm in 484e3be Feb 22, 2018

initialisation in the BEGIN sub attempts to use 'default' as 'HashRef' which errors when it sees empty string.
Changed as part of expanding test coverage for subs

@ThornyS

This comment has been minimized.

Owner

ThornyS commented on lib/GraphViz2.pm in 484e3be Feb 22, 2018

podchecker spotted a failed link as a result of what looks like a typo L</logger($logger_object)> , so removed trailing ']'

@ThornyS

This comment has been minimized.

Owner

ThornyS commented on lib/GraphViz2/Filer.pm in 484e3be Feb 22, 2018

Set the head(n) hierarchy order spotted by podchecker.

@ronsavage ronsavage merged commit 7007fd3 into ronsavage:master Feb 22, 2018

@ronsavage

This comment has been minimized.

Owner

ronsavage commented Feb 23, 2018

V 2.47 is now on CPAN. See the Changes file for some of the details.

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