Skip to content
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

[2336] Fix API for DotCover.report #2389

Merged
merged 3 commits into from
Oct 10, 2019

Conversation

TomasMorton
Copy link

@TomasMorton TomasMorton commented Aug 31, 2019

Description

As per #2336, fixes the definition of the DotCover.report function, as it was not handling the failBuild param of buildParamsAndExecute, added in #990.

Note that this would be a "breaking change" for anyone already using the module, because they will be passing in the parameter themselves.

TODO

@matthid matthid added the needs-tests This PR needs tests in order to be accepted label Sep 5, 2019
@matthid
Copy link
Member

matthid commented Sep 5, 2019

Nice catch!
While technically a breaking change, we will revert this (just as you suggest in this PR) as I indeed consider this a bugfix.
Please also feel free to use failbuild=true. We can always set to false later, but if we already break we can fix that default as well (maybe even for the DotCoverMerge function?). This was basically the pre 2015 behavior...

@matthid matthid merged commit 3d65cf8 into fsprojects:release/next Oct 10, 2019
@TomasMorton
Copy link
Author

Hey @matthid,

Really sorry, I've been too busy to get around to making modifications. I agree it would have made more sense to have fail as true.

Thank you for merging the fix.

@TomasMorton TomasMorton deleted the 2336/fix-dotcover branch October 10, 2019 21:06
@matthid matthid mentioned this pull request Oct 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-tests This PR needs tests in order to be accepted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants