Skip to content
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

Fix gradle build and clean issue on Windows. #84

Merged
merged 1 commit into from
Sep 8, 2017
Merged

Fix gradle build and clean issue on Windows. #84

merged 1 commit into from
Sep 8, 2017

Conversation

AndrewPolovko
Copy link
Contributor

I am facing the following issue on my windows machine: after building the project I am unable to clean it because windows refuses to delete ktlint checkstyle report.

https://gist.github.com/anonymous/6e2b55fe13ab806880faf0fc0dd70cba

It seems that this file is being locked by plugin code.
In my fix I deleted FileOutputStream that seems to be unnecessary.
I verified it on my machine.

@codecov
Copy link

codecov bot commented Sep 1, 2017

Codecov Report

Merging #84 into master will decrease coverage by 0.9%.
The diff coverage is 60%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #84      +/-   ##
============================================
- Coverage     59.19%   58.28%   -0.91%     
  Complexity       31       31              
============================================
  Files             2        2              
  Lines           174      175       +1     
  Branches         22       22              
============================================
- Hits            103      102       -1     
- Misses           68       70       +2     
  Partials          3        3
Impacted Files Coverage Δ Complexity Δ
...h/code/quality/tools/CodeQualityToolsPlugin.groovy 57.3% <60%> (-0.93%) 31 <0> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2f64466...4b8d434. Read the comment docs.

Copy link
Owner

@vanniktech vanniktech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure standardOutput is a property of the task and the content will then be written into the file. How does this work now?

@AndrewPolovko
Copy link
Contributor Author

Yeah, you are right. It fixed problem but wasn't generating reports properly.

In second commit I moved file creation to doFirst{} block, this way problem is also fixed, all reports are working and most importantly files will be created only during gradle execution phase, and not as before – on configuration phase.

@vanniktech
Copy link
Owner

I'm at the conference at the moment and will look soon.

Copy link
Owner

@vanniktech vanniktech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nits then I'll merge

ignoreExitValue = true

doFirst {
new File(outputDirectory).mkdirs()
standardOutput = new FileOutputStream("$outputFile")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be just new FileOutputStream(outputFile)

@@ -244,13 +244,16 @@ class CodeQualityToolsPlugin implements Plugin<Project> {

subProject.task('ktlint', type: Exec) {
commandLine 'java', '-cp', subProject.configurations.ktlint.join(System.getProperty('path.separator')), 'com.github.shyiko.ktlint.Main', '--reporter=checkstyle', 'src/**/*.kt'
def outputDirectory = new File(subProject.buildDir, "reports/ktlint/")
outputDirectory.mkdirs()
def outputDirectory = "${subProject.buildDir}/reports/ktlint"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's do reports/ktlint/ here

def outputDirectory = new File(subProject.buildDir, "reports/ktlint/")
outputDirectory.mkdirs()
def outputDirectory = "${subProject.buildDir}/reports/ktlint"
def outputFile = "$outputDirectory/ktlint-checkstyle-report.xml"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and remove the / here

@AndrewPolovko
Copy link
Contributor Author

Done

@@ -244,13 +244,16 @@ class CodeQualityToolsPlugin implements Plugin<Project> {

subProject.task('ktlint', type: Exec) {
commandLine 'java', '-cp', subProject.configurations.ktlint.join(System.getProperty('path.separator')), 'com.github.shyiko.ktlint.Main', '--reporter=checkstyle', 'src/**/*.kt'
def outputDirectory = new File(subProject.buildDir, "reports/ktlint/")
outputDirectory.mkdirs()
def outputDirectory = "${subProject.buildDir}reports/ktlint"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing the / at the end

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

-  ktlint: create report filed during execution phase
Copy link
Owner

@vanniktech vanniktech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm on mobile right now and will test this locally before merging. Thanks already 👍

@vanniktech
Copy link
Owner

Sorry it took so long. Thanks for the fix 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants