Skip to content

Improve gradle spotless apply suggestion #578

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

Merged
merged 7 commits into from
May 15, 2020

Conversation

breskeby
Copy link
Contributor

This PR adds a minor improvement to the suggestion message

Run 'gradlew spotlessApply' to fix these violations.

to

  1. use a OS specific invocation of gradlew
  2. provide the full task path of the spotlessApply task

which results in

// windows
Run 'gradlew.bat :spotlessApply' to fix these violations.

// unix, osx, linux
Run './gradlew :spotlessApply' to fix these violations.

Especially for larger multi project builds, this makes copy pasting the suggested fix easier and more efficient as it only runs on the problematic project and not on all projects in the multi project build.

@breskeby breskeby changed the title Improve spotless apply suggestion Improve gradle spotless apply suggestion May 14, 2020
Copy link
Member

@nedtwigg nedtwigg left a comment

Choose a reason for hiding this comment

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

Thanks! A few small problems noted below. Main problem is that our CI didn't run on this PR, which I believe I just fixed.

I assume that you are working on a very large project. Does it happen often that you have problems in two+ projects? The ultimate form of this PR would be to somehow aggregate failures across all invocations of spotlessCheck, and combine it into one error message. I'm definitely not asking for that PR, and infact we would reject it at the moment because we need to merge #576 first, but I am curious if you feel you would benefit from it or not.

@breskeby
Copy link
Contributor Author

I was thinking if gathering all tasks end have one message that tells you all the paths to a apply would be a good solution here. In general that would only apply if you ran gradle with --continue or maybe with running --parallel. So far me personally never hit the issue of having many different spots to fix at once but there is a use case for that. With this solution here, you now would get one suggested fix per failing task (project) with the appropriate fix. Given the simple implementation that is a good compromise I think.
One option could be to move the messaging to the buildFinished hook and check all the failing tasks and then provide an even better suited message (e.g. based on how many spotless tasks have failed or how big the project is). But I'm not sure if this is worth the added complexity tbh

@breskeby breskeby force-pushed the improve-spotlessApply-suggestion branch from 0ce547d to 3008178 Compare May 15, 2020 08:09
@breskeby
Copy link
Contributor Author

I've rebased and updated the PR against latest master

Copy link
Member

@nedtwigg nedtwigg left a comment

Choose a reason for hiding this comment

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

Gonna let this breathe for a couple days before merging and releasing.

@nedtwigg
Copy link
Member

The OS fix makes it easier to merge :)

@nedtwigg nedtwigg merged commit 9a761d7 into diffplug:master May 15, 2020
@nedtwigg
Copy link
Member

Released in plugin-gradle 4.0.0

@nedtwigg
Copy link
Member

I don't know how to detect this, but Windows PowerShell requires .\gradlew or .\gradlew.bat, the suggestion here won't work. Doesn't bother me, just an FYI for any interested parties :)

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.

2 participants