-
-
Notifications
You must be signed in to change notification settings - Fork 756
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
Merge XML report output #3491
Merge XML report output #3491
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3491 +/- ##
============================================
+ Coverage 80.06% 80.51% +0.44%
- Complexity 2864 2893 +29
============================================
Files 458 461 +3
Lines 8584 8668 +84
Branches 1663 1676 +13
============================================
+ Hits 6873 6979 +106
+ Misses 794 767 -27
- Partials 917 922 +5
Continue to review full report at Codecov.
|
detekt-gradle-plugin/src/intTest/kotlin/io/gitlab/arturbosch/detekt/DetektXmlReportMergeTest.kt
Show resolved
Hide resolved
detekt-gradle-plugin/src/main/kotlin/io/gitlab/arturbosch/detekt/report/XmlReportMergeTask.kt
Outdated
Show resolved
Hide resolved
detekt-gradle-plugin/src/main/kotlin/io/gitlab/arturbosch/detekt/report/XmlReportMergeTask.kt
Show resolved
Hide resolved
…kt/report/XmlReportMergeTask.kt Co-authored-by: Nicola Corti <corti.nico@gmail.com>
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.
I really like this feature. I had a short look at it. It works fine. Great! 👍
I think it's good to start small with the XML report format. For me XML reports supporting a merge is the first logical step. Other reports could follow later on, if this is highly valuable.
I'm not sure regarding the test coverage for the XML parsing logic and task. Should we include more edge cases?
detekt-gradle-plugin/src/intTest/kotlin/io/gitlab/arturbosch/detekt/DetektXmlReportMergeTest.kt
Show resolved
Hide resolved
Co-authored-by: M Schalk <30376729+schalkms@users.noreply.github.com>
The logic is relatively straightforward so I think the existing test coverage should be fine and jacoco is happy about hitting the 80% coverage in this diff. The overall project test coverage is increased, but the patch diff test coverage is lowered. We probably need to look into why it is giving us conflicting test coverage. |
This is a rewrite of #3394 and addresses one item in #3360.
utf-8
toUTF-8
, because the latter is the standard.