-
-
Notifications
You must be signed in to change notification settings - Fork 783
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
Add a new line at the end of the txt report #2255
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2255 +/- ##
============================================
+ Coverage 81.26% 81.45% +0.18%
- Complexity 2101 2114 +13
============================================
Files 348 349 +1
Lines 6020 6054 +34
Branches 1102 1106 +4
============================================
+ Hits 4892 4931 +39
+ Misses 534 532 -2
+ Partials 594 591 -3
Continue to review full report at Codecov.
|
@@ -11,6 +11,8 @@ class TxtOutputReport : OutputReport() { | |||
|
|||
override fun render(detektion: Detektion): String { | |||
val smells = detektion.findings.flatMap { it.value } | |||
return smells.joinToString("\n") { it.compactWithSignature() } | |||
return smells.joinToString("\n") { it.compactWithSignature() }.let { |
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.
Sorry for messing around with your branch, but isn't it enough to add just one new line at the line of the file to be compliant with git?
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.
Yes, and this is what this code does... If you mean this: smells.joinToString { it.compactWithSignature() + "\n" }
it would work too(out was my first impregnation). But we are losing the performance of string builder. That would create 2k useless strings.
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.
Uh sorry. By looking at the test code once more, it becomes clear what this code is intended to do.
But something like this should work without introducing a new branch.
return smells.joinToString("\n") { it.compactWithSignature() }.let { | |
val builder = StringBuilder(); | |
smells.forEach { builder.append(it.compactWithSignature()).append('\n') } | |
return builder.toString() |
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.
Agree! This is easier.
TestSmellC - [TestEntity] at TestFile.kt:1:1 - Signature=S1 | ||
|
||
""".trimIndent() |
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.
Here we have two new lines at the end of the file.
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.
trimIndent
removed the second end of line. I can use trimMargin
to make it easier to understand.
This PR adds a
\n
at the end of the txt report. This way you can concat all the reports easily. Or you can do something likewc -l report.txt
to count the number of errors that you and not be off by oneContext
This is a real minor thing but I lose 10 minutes today because of this. I was counting the errors of different branches using
wc -l report.txt
. And I was getting always one error less than expected. This PR will avoid that problem.