parroty / excoveralls

Coverage report tool for Elixir with coveralls.io integration.
MIT License
820 stars 178 forks source link

Always floor coverage instead of rounding #310

Closed albertored closed 11 months ago

albertored commented 1 year ago

We do not want to report a 100% coverage when there are lines that are not covered.

albertored commented 1 year ago

@parroty any news on this?

albertored commented 1 year ago

PS: @parroty this is an improvement over #257 covering all the cases where percentages are calculated and formatted, if merged #257 can be closed

parroty commented 1 year ago

Thanks for the follow-up 🙇 . Please let me separate this item from the current v0.17.0, as I would like to clarify the compatibility (breaking) portion of this change.

tielur commented 11 months ago

@parroty interested when this is going out to hex, we are running into this on our CI reporting 100% even though it's not actually 100%

parroty commented 11 months ago

I'm sorry about not responding to this... I have some concerns around the compatibility part. Could anyone help adding this as configuration option, as opt-in? 🙇

albertored commented 11 months ago

Hi @parroty, I understand your concerns about being retro-compatible but I think the actual behaviour is conceptually wrong (showing 100% coverage when some lines are not covered). Are you open to adding a configuration option to restore the previous behaviour and make the floor coverage the default one? I.e. having an opt-out option instead of opt-in as you suggested

parroty commented 11 months ago

Are you open to adding a configuration option to restore the previous behaviour and make the floor coverage the default one? I.e. having an opt-out option instead of opt-in as you suggested

Thank you for the comment 🙇 . I was originally thinking about maintaining the current behavior (opt-in for a floor coverage change), but it may be acceptable for the new settings for keeping the behavior considering your point (by noting it as a behavioral change in Changelog).

albertored commented 11 months ago

Perfect, I'll add the option to this PR in a couple of days, thank you

albertored commented 11 months ago

@parroty ready

parroty commented 11 months ago

Thank you! (I'll be publishing the new version later).