Add output report and report type. #18
Conversation
Now plugin outputs check task results to build/reports/ktlint/ folder. Also user can select one of supported report types. Signed-off-by: Yahor Berdnikau <egorr.berd@gmail.com>
|
One of the minor problems so far - now there is no plugin output in the console. User needs to open report file to see the errors. I've tried to use |
|
Opened an issue for it: pinterest/ktlint#71 |
| /** | ||
| * Report output format. | ||
| * | ||
| * Possible values are: plain, checkstyle, json. |
There was a problem hiding this comment.
Do want to turn this into an enum?
There was a problem hiding this comment.
Edit: probably not in case more are added and this plugin isn't updated.
There was a problem hiding this comment.
Can you set it to multiple output formats or only one at a time?
There was a problem hiding this comment.
Only one at a time.
Enum may be not a bad idea, but harder to maintain - like adding a new one and check supported version.
There was a problem hiding this comment.
Put supported version in the enum along with the file extension.
There was a problem hiding this comment.
I've introduced reporter enum. See 5fd454c.
|
|
||
| private fun createCheckTaskOutputDir(target: Project, extension: KtlintExtension): File { | ||
| val reportsDir = File(target.buildDir, "reports/ktlint") | ||
| reportsDir.mkdirs() |
There was a problem hiding this comment.
Can you use org.gradle.util.GFileUtils.mkdirs instead?
There was a problem hiding this comment.
Fixed in 82ec747.
| "checkstyle" -> "xml" | ||
| "json" -> "json" | ||
| else -> "txt" | ||
| } |
There was a problem hiding this comment.
What if a different output format is added?
There was a problem hiding this comment.
Then txt will be used. Again I can introduce enum for this reporters and define there file extension.
There was a problem hiding this comment.
Now plugin uses enum for defining possible variants.
| private fun addReporter(extension: KtlintExtension, runArgs: MutableSet<String>) { | ||
| if (isReportAvailable(extension.version)) { | ||
| runArgs.add("--reporter=${extension.reporter}") | ||
| } |
There was a problem hiding this comment.
Log a debug message in the else case. It isn't really a warning but it may help someone trying to debug why their isn't a report showing up.
There was a problem hiding this comment.
Fixed in 82ec747.
Signed-off-by: Yahor Berdnikau <egorr.berd@gmail.com>
Apply reporter in one place - on check task creation. Signed-off-by: Yahor Berdnikau <egorr.berd@gmail.com>
|
@JLLeitschuh please review updated PR. |
| this.args("--reporter=${extension.reporter.reporterName}") | ||
| this.standardOutput = createReportOutputDir(target, extension).outputStream() | ||
| } else { | ||
| target.logger.info("Reporter is not available in this ktlint version") |
There was a problem hiding this comment.
Perhaps making this level debug would be more appropriate? What do you think?
There was a problem hiding this comment.
I've also initially thought about debug level, but gradle, on debug level, produces too much output. This message can be missed. And this message, IMHO, is more informational, then debug.
|
Are you happy with where this is? Should this be merged? |
|
@JLLeitschuh yes, it is ready to be merged. Will do improvement when ktlint will add reporter output param. |
Now plugin outputs check task results to build/reports/ktlint/ folder.
Also user can select one of supported report types.
Fixes #13
Signed-off-by: Yahor Berdnikau egorr.berd@gmail.com