-
Notifications
You must be signed in to change notification settings - Fork 53
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
Implemented aggregate reports #90
Conversation
Great news :) was waiting for this actual feature so much. |
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.
It's rather list of questions than an actual review, but we have to start from something
src/functionalTest/kotlin/kotlinx/kover/test/functional/cases/utils/Commons.kt
Outdated
Show resolved
Hide resolved
|
||
internal const val DEFAULT_XML = "reports/kover/report.xml" | ||
internal const val DEFAULT_HTML = "reports/kover/html" | ||
internal fun defaultXmlReport() = "reports/kover/report.xml" |
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.
Why these are functions, not val
s?
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.
This is done because at the moment there is no certainty that additional parameters will not be needed to get the default value (for example, as in the case of defaultBinaryReport). So it turns out the general style that getting all the values is a functions, and not a mixture of val
and fun
.
However, I do not insist on such a decision, if val
is perceived better, I can change it.
Co-authored-by: Leonid Startsev <[email protected]>
Co-authored-by: Leonid Startsev <[email protected]>
Co-authored-by: Leonid Startsev <[email protected]>
Resolves #20, #43