Packages with leading underscores not indexed

[andreeap]

A fix for #358, not ready to merge.

[coveralls]

Coverage decreased (-0.06%) to 88.51% when pulling 1546b05 on andreeap:issue-358 into dc68100 on CPAN-API:master.

[coveralls]

Coverage decreased (-0.06%) to 88.51% when pulling 1546b05 on andreeap:issue-358 into dc68100 on CPAN-API:master.

[coveralls]

Coverage decreased (-0.14%) to 88.43% when pulling 1546b05 on andreeap:issue-358 into dc68100 on CPAN-API:master.

[andreeap]

This is ready for merge now.

[rwstauner]

It isn't obvious why the asterisk is here... it makes me wonder if that was intentional.
Instead of special casing some exclusions (leading underscore), perhaps it would be better to use the same regexp that PAUSE does and simply invert the check:
if( $mod->name !~ /^[A-Za-z]/ ){ $mod->indexed(0) }

I would additionally then comment on why we're doing this check (where the regexp came from (link to the PAUSE source code)).

The test should still pass, but the code will be clearer and more representative of where the "rule" came from.

[andreeap]

@rwstauner This makes more sense than what I tried. I made the change and I also added a little comment for clarification. I hope it is ok now. Many thanks for feedback.

[coveralls]

Coverage increased (+0.01%) to 88.58% when pulling a87e417 on andreeap:issue-358 into dc68100 on CPAN-API:master.

[rwstauner]

Yeah, I think that looks better. Thanks.

If you comment out your new code in the lib, does your test fail?
If not, then the test isn't testing what you think it is.

[andreeap]

If I comment that, it will fail, but otherwise, how can I check the package name exactly?

[rwstauner]

@andreeap Can you pull your master branch and then rebase this branch against that?
I refactored that test file and added a helper function to make it more intuitive and reduce duplication (and confusion).

After rebasing, replace
MetaCPAN::Document::File->new( %stub,
with
new_file_doc(
also remove the set_indexed line (it's no longer necessary).

Then you'll see that the test no longer passes.

There's a very simple bug in the lib code, but the test isn't showing you that b/c the current test doesn't contain file content, and the doc won't index a module that doesn't have a package statement in the content.
Unfortunately there were some bad examples in the test file that might have tricked you, so I tried to clean that up a bit.

[andreeap]

@rwstauner Yes, I will rebase and make the changes and indeed I looked at the other tests for examples.

[andreeap]

The module still gets indexed, which means that I still don't check what am I supposed to?

[rwstauner]

Ok, so now that the test is working properly, you can see that the code is not.

In the lib code you compare the package to the regexp, and if it matches, set indexed 0,
but then the code continues to the next line and overwrites the value.

Inside your if block you could put a next; after the $mod->indexed(0); to move to the next item in the loop since you don't need to do any more processing on this one.

[coveralls]

Coverage decreased (-0.06%) to 88.58% when pulling 141117f on andreeap:issue-358 into e4981c7 on CPAN-API:master.

[andreeap]

I can see it now and I feel stupid for this mistake, but at least next time I will pay extra attention.

[rwstauner]

Happens to us all ;-)
That's how we learn.

Thanks for doing this!

[andreeap]

Thank you for helping me :)