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

com.diffplug.spotless 5.0 #640

Merged
merged 35 commits into from
Jul 12, 2020
Merged

com.diffplug.spotless 5.0 #640

merged 35 commits into from
Jul 12, 2020

Conversation

nedtwigg
Copy link
Member

@nedtwigg nedtwigg commented Jul 5, 2020

This PR closes out #600. It's much simpler than the long list of commits looks. All I did was:

  • delete everything which was deprecated (had already been logging deprecation warnings)
  • renamed SpotlessExtensionBase -> SpotlessExtension, and SpotlessExtension(Modern -> Impl)
  • renamed SpotlessTaskBase -> SpotlessTask, and SpotlessTask(Modern -> Impl)
  • changed plugin id to com.diffplug.spotless (lets us set new tags)
  • old plugin id com.diffplug.gradle.spotless now gives this error message:
We have moved from 'com.diffplug.gradle.spotless'
                to 'com.diffplug.spotless'
To migrate:
- Test your build with: id 'com.diffplug.gradle.spotless' version '4.5.1'
- Fix any deprecation warnings (shouldn't be many / any)
- Now you can use:      id 'com.diffplug.spotless' version '5.0.0'

That's all you really need to know, but as always, there are more details in the changelog:
https://github.com/diffplug/spotless/blob/main/plugin-gradle/CHANGES.md

While you're at it, you might want to search for "target '**/".  We used
to  recommend that in our README, but it's a lot slower than something
more specific like "target 'src/**".  Also, if you haven't tried them yet,
take a look at our IDE integration and 'ratchetFrom'.  We've found them
to be useful, hope you do too.

If you like the idea behind 'ratchetFrom', you should checkout spotless-changelog
https://github.com/diffplug/spotless-changelog

I'm open to feedback on anything, but I think the main decision points are:

  • the id migration described above, and what we want to tell users
  • the decision to keep SpotlessExtension and SpotlessTask in a separate API vs impl piece.

We could smush them back together, but I thought that the "spotlessModern" refactor where we had both implementations side-by-side was useful. Since we did the work to split them up, we might as well keep them that way, but I'm open to other thoughts.

…entation details live in `SpotlessExtensionImpl`.
@nedtwigg nedtwigg requested review from bigdaz and jbduncan July 5, 2020 08:31
@nedtwigg
Copy link
Member Author

nedtwigg commented Jul 7, 2020

Edit 7/7/20: added recommendation to the redirect message above about the bad performance of "target '**/"

@nedtwigg
Copy link
Member Author

This is a big PR, and people are busy, no worries :). I plan to merge and release this tonight. If you would like me to delay, feel free to ask for more time. The code change is very simple, just deleting deprecated things. The main thing that would benefit from a second pair of eyes is the migration message above, but having sat on it for a week, I feel pretty good about it.

@thc202
Copy link
Contributor

thc202 commented Jul 10, 2020

Shouldn't the **5.x preview:** note be removed now? And the (TODO)?

@nedtwigg
Copy link
Member Author

Shouldn't the 5.x preview: note be removed now? And the (TODO)?

Yes, good catch! Thanks!

@nedtwigg nedtwigg merged commit 2f32f0d into main Jul 12, 2020
@nedtwigg nedtwigg deleted the feat/modern branch July 12, 2020 21:18
@nedtwigg
Copy link
Member Author

Released, thanks @thc202!

@bigdaz
Copy link
Contributor

bigdaz commented Jul 21, 2020

Great job on all of the Spotless improvements for Spotless 5.x. It's great that we can now leverage the newer features of Gradle. Thanks @nedtwigg !!!

@bigdaz
Copy link
Contributor

bigdaz commented Jul 21, 2020

Oh, and thanks for taking on the work to make the task inputs relocatable. I've done some rudimentary testing and it looks good so far. I'll report back any issues that I see in the wild.

@nedtwigg
Copy link
Member Author

Thanks for the help getting us here! The new task layout you implemented is much better.

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