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
Clean Coverage, no longer used #2866
Conversation
Signed-off-by: Omar Shrit <omar@shrit.me>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you remove the badge from the https://github.com/mlpack/mlpack/blob/master/README.md as well?
Of course, I will remove the badge. |
Signed-off-by: Omar Shrit <omar@shrit.me>
We don't, don't mind to remove the script as part of the PR as well. |
Signed-off-by: Omar Shrit <omar@shrit.me>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, though I may have missed some conversation on removing versus fixing coverage builds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thanks for taking the time to do this. I see there's a reference to BUILD_WITH_COVERAGE
in doc/guide/build.hpp
---do you think you can remove that too?
@birm basically the coverage build has been failing for a very long time and doesn't have an "owner", so we thought it was reasonable to remove it. (If anyone feels strongly and wants to bring it back, that's totally ok! I just don't think anyone does. :))
Signed-off-by: Omar Shrit <omar@shrit.me>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, looks good to me. 👍
Signed-off-by: Omar Shrit omar@shrit.me