garu / App-cpanminus-reporter

stand-alone CPAN Testers client (for cpanminus and friends)
13 stars 13 forks source link

Not all installations are being reported now #26

Closed 0x1F602 closed 8 years ago

0x1F602 commented 9 years ago

Hi there, I'm doing this as part of the PR challenge.

There's a bug described in https://rt.cpan.org/Public/Bug/Display.html?id=103866

I believe this bug is actually in CPAN-Testers-Common-Client.

I'm looking at commit ffa25a413ae3dbee7adce451a88eae7754e98fbb and not finding anything out of place. It'd be nice if the history checking was opt-in instead of opt-out.

My belief is that the issue is in $client->duplicate. I'm justifying my belief on the basis that the only output ether saw was "sending: ", which is triggered around here: https://github.com/garu/App-cpanminus-reporter/blob/9c6b6d9efd0bfb41f89b02ce35247e8d53679a88/lib/App/cpanminus/reporter.pm#L372

garu commented 8 years ago

Hi @beta0x64! Thank you for opening this discussion.

The reason why it's opt-out instead of opt-in is because duplicate reports are not just useless but can actually give the wrong impression on a module (e.g. a thousand PASS on an architecture/perl versions when they are actually all just the same test run).

I really appreciate the second pair of eyes taking a peek at the source code to see if there's anything wrong. I have made some changes on the parser and talked to karen (who filed the original ticket) and it looks like the issue went away, so I'm closing this one as well.

If you have any questions or concerns, please do let me know.

Cheers!