-
-
Notifications
You must be signed in to change notification settings - Fork 732
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
Added DotCover Cover command #713
Conversation
@mholo65 Nice! 👍 Funny, but as you published this PR I was updating the site to show inherited members better (cake-build/website#65) 😄 As soon as that PR is merged, this won't be a problem anymore. As long as it's not a breaking change, the refactoring is OK with me. I would however recommending adding an abstract Great job! |
Nice! Shouldn't be a breaking change, Analyse uses all settings as Coverer plus ReportType. |
987ab40
to
c145a8a
Compare
@patriksvensson, refactored, rebased and done. An additional suggestion for removing duplicate code.
DotCoverWhateverName<TSettings> : DotCoverTool<TSettings> where TSettings : DotCoverSettings
internal static ProcessArgumentBuilder GetArguments(this DotCoverSettings settings, ProcessArgumentBuilder builder) What do you think? Or should we leave it as is. |
|
||
/// <summary> | ||
/// Gets the coverage filters using the following syntax: +:module=*;class=*;function=*; | ||
/// Use -:myassembly to exclude an assembly from code coverage. |
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.
Trim trailing white space here.
@patriksvensson looks good to me, just left one minor comment, remove WIP and rebase, also do we have an referring issue to the PR assigned to a release? |
@devlead Doh! Assumed there was an issue for this. @mholo65 Can you create an issue for this? |
@mholo65 Perfect, thanks! |
c145a8a
to
c7e6863
Compare
Trimmed, rebased and done. @devlead You probably meant the trailing whitespace in DotCoverSettings.cs:35 not DotCoverAnalyseSettings.cs:35 since You are referring to an deletion in your comment. |
HOLD!. sorry, forgot to rebase :) |
c7e6863
to
a0c88b3
Compare
Rebased and checks have passed. |
Merged! Thank you for this! 👍 |
Added DotCover Cover command. Needed since TeamCity only want snapshots, not HTML/XML reports.
If possible, to remove duplicate code, I would like to reuse DotCoverCoverSettings in DotCoverAnalyser by having DotCoverAnalyseSettings to inherit from DotCoverCoverSettings. Is this allowed since it will make documentation unclear?
NOTE: Quickly hacked using my Linux box, need to verify functionality on Windows (dotCover.exe doesn't run under Mono). But this have to wait until monday when I'm back at work.
//cc @patriksvensson