Closed JJ closed 7 years ago
Address issue #50 . This is the suggestion I received.
2017-05-28 19:49 GMT+02:00 Karen Etheridge notifications@github.com:
@karenetheridge commented on this pull request.
As long as we support these perl versions we should continue to test them on travis.
The problem is that they error with the tooling needed for coveralls; the actual code works without a problem. I don't know if it can be fixed in the .travis.yml; my suggestion would be to add CI in another site.
We can run the code coverage tests on just one version of perl (say 5.22 or 5.24) while running the user-facing tests across all perls that we support. We should not sacrifice the latter to get the former.
On Sun, May 28, 2017 at 10:52 AM, Juan Julián Merelo Guervós < notifications@github.com> wrote:
2017-05-28 19:49 GMT+02:00 Karen Etheridge notifications@github.com:
@karenetheridge commented on this pull request.
As long as we support these perl versions we should continue to test them on travis.
The problem is that they error with the tooling needed for coveralls; the actual code works without a problem. I don't know if it can be fixed in the .travis.yml; my suggestion would be to add CI in another site.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/libwww-perl/LWP-Protocol-https/pull/51#issuecomment-304529686, or mute the thread https://github.com/notifications/unsubscribe-auth/AASfy2WIo5YED4ylk58lewQ5-5UgvJclks5r-bRKgaJpZM4Nos23 .
2017-05-28 19:54 GMT+02:00 Karen Etheridge notifications@github.com:
We can run the code coverage tests on just one version of perl (say 5.22 or 5.24) while running the user-facing tests across all perls that we support. We should not sacrifice the latter to get the former.
That's exactly what that file does. But it installs tooling independently of the value of the environment variable. I can try and see if I can install the coverall tooling conditionally based on that variable. In fact, Moo and Moose seem to do it that way. I can try that.
It's testing OK now. I only had to change the copyright year in dist.ini. The gist of it is that there's some rule in dist.ini that makes it check that, and that using build-dist was triggering installation of Dist::Zilla and rules, while the previous Travis CI wasn't doing that. Boils down to: it's got coverage testing now, and it also tests for all versions of perl previously used.
Squashed into two commits, most of the things in the second one. Changes requested done.
Thanks very much @JJ!
Thanks!
2017-06-04 18:15 GMT+02:00 Olaf Alders notifications@github.com:
Merged #51 https://github.com/libwww-perl/LWP-Protocol-https/pull/51.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/libwww-perl/LWP-Protocol-https/pull/51#event-1109167346, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAB9PFA5tCPzfzie-im69pJYoOkulaSks5sAtg_gaJpZM4Nos23 .
-- JJ
Following the example of @moose, but it's pretty much the same you pointed me to. For the time being, it prints coverage in the logs (spoiler: not good) but I guess that if it finds a matching repo in coveralls.io, which you said you had set up, it should upload it there. Tell me if it does not work