excoveralls
excoveralls copied to clipboard
parroty
Add a CLI flag to silence warnings from `:cover` module
This introduces a new CLI flag --no-warn-cover, which will silence the warnings that we get from :cover.
Those warnings are often the result of using mocks (especially if you're using :meck), and therefore
can be ignored.
Unfortunately there's no flag or anything that turns this off in the default :cover module, which is why
we need to use our own copy of that module here. Luckily that module has barely changed in over 5
years, and so it's stable and safe to copy in this fashion.
Resolves #230
Thanks for the PR. The cover.erl is coming from the erlang modules? Thank you for the description about compatibility, but I have some concerns that whether if this change has some implications regarding license term (and wondering if it can be applied as optional component).
I'm searching around,
- https://softwareengineering.stackexchange.com/questions/297217/using-code-under-apache-2-0-license-in-my-program-using-mit-license
- https://law.stackexchange.com/questions/6081/can-i-bundle-mit-licensed-components-in-a-apache-2-0-licensed-project
Yep, that code comes from Erlang. I'm no lawyer, but if there's some reason you can't include that code here then I totally understand. That is unfortunately the only way to solve this issue, though, so if this doesn't work then this issue will need to be closed and marked as something that cannot be solved.
I'm facing this annoying warning but I still can't figure out the real reason of this message.
@parroty just needs attribution as both MIT and Apache 2.0 are permissive - as long as individual source files include the license and attribution. I'd like to see this get merged.
We've been running into this as well. It's terribly annoying.
It looks like lib/tools/src/cover.erl has received some fixes since this was opened. Some in pernicious edge cases.
I don't like the idea of vendoring a modified version. We could upstream changes that introduce :cover.start/1, which would allow us to silence logs (silence: true).
@parroty any chance we can merge this PR soon or if you disagree close this as "won't fix" ?
Thank you for the follow-up. I didn't have good timing to dig this up, and sorry about the lack of responses.
just needs attribution as both MIT and Apache 2.0 are permissive - as long as individual source files include the license and attribution. I'd like to see this get merged.
I am not confident about this licensing part enough yet. Is it possible for you to elaborate this "needs attribution" part? (What needs to be included in this repository?). Thanks in advance.
https://www.erlang.org/EPLICENSE
3.5. Required Notices.
You must duplicate the notice in Exhibit A in each file of the Source
Code, and this License in any documentation for the Source Code, where
You describe recipients' rights relating to Covered Code. If You
created one or more Modification(s), You may add your name as a
Contributor to the notice described in Exhibit A.
It's my understanding that prefixing the modified cover.erl with the EPLICENSE text as a comment would be sufficient.
I don't like the idea of vendoring a modified version. We could upstream changes that introduce :cover.start/1, which would allow us to silence logs (silence: true).
I'd like that, much better option if we can get a patch upstream.
FWIW, I've got around the issue by patching cover.erl to remove the warnings and then added that under src/ in my project.
@deepfryed Thank you for the comment.
It's my understanding that prefixing the modified cover.erl with the EPLICENSE text as a comment would be sufficient.
The file in this PR (quiet_cover.erl) maintains the license section (comment lines), so just merging this PR satisfies the requirement? (Does this statement match with your understanding?)
I don't like the idea of vendoring a modified version. We could upstream changes that introduce :cover.start/1, which would allow us to silence logs (silence: true). I'd like that, much better option if we can get a patch upstream.
I am not in favor of vendoring a modified version..., but if it can avoid license issues, I am inclined to include this feature with experimental or tentative types of notes. hmm.
FWIW, I've got around the issue by patching cover.erl to remove the warnings and then added that under src/ in my project.
Also, this can be a reasonable workaround?
@parroty I don't think that people should have to patch cover.erl to remove these warnings. The PR is here, why not just merge it? I get this all the time when using Mock, which is a fundamental part of testing to begin with.
As an alternative, we could add how to manually patch cover.erl in README.md, but IMHO it's a feature that should be a part of the official package. It's reasonable to be able to silence these warnings, especially because unnecessary console output drastically slows down test suites.
Wouldn't the right approach here be to first upstream some changes to :cover?
https://github.com/erlang/otp/pulls
It doesn't seem right to copy all of that here.
Wouldn't the right approach here be to first upstream some changes to :cover? https://github.com/erlang/otp/pulls
Could anyone give a hand for this part? 🙇 Merging of this PR itself might be one solution, but it would be only for short-term (temporary) one.
As an alternative, we could add how to manually patch cover.erl in README.md, but IMHO it's a feature that should be a part of the official package. It's reasonable to be able to silence these warnings, especially because unnecessary console output drastically slows down test suites.
Also, it would be great if anyone can PR this README option, assuming that there's no harm writing the procedure as temporary solution 🙇 .
FWIW, I think cover.erl patch belongs in OTP. I'd be fine with someone submitting that patch upstream and instructions in README in the interim that can guide people on adding a patched cover.erl to src/ in their projects.
Wouldn't the right approach here be to first upstream some changes to :cover? https://github.com/erlang/otp/pulls
Yes
it would be great if anyone can PR this README option, assuming that there's no harm writing the procedure as temporary solution
I'll send you a PR tomorrow to review, unless someone else beats me to it.