-
Notifications
You must be signed in to change notification settings - Fork 68
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Minimum requirement #73
base: master
Are you sure you want to change the base?
Conversation
#71 is now merged, could you adapt this PR to be based on the current Also, I think users would expect the minimum requirement % calculation to include the untested files when |
Can I open another PR with the change? |
Is this still in progress. Would love to get this feature landed |
@allbarbos Yes, please! (I'm very low on bandwidth for maintaining LuaCov — in fact, the previous maintainer passed away and I'm carrying the torch, but I'd love to get some maintainership help on this! If any of you wants to help out, please update this PR and let me know and I'll grant commit access) |
e77feff
to
1b9a85a
Compare
@hishamhm I fixed up the PR. But it has issues beyond the simple feature;
A simple option would be to leave this as it is today, and have a CI user run 2 reports. First the one they want for display/reporting. And then the default one, which will display the error. (though that means one cannot configure the stats files to be cleared, since the second run would need those as well) |
I'm leaning towards a separate reporter that doesn't really report anything, but just generates the error message. |
Added an option to put a minimum coverage constraint.
This resolves issue #68