Repository navigation
Spotless plugin-gradle "modern" #600
Description
Activity
- pinned this issue
on Jun 5, 2020 - unpinned this issue
on Jun 5, 2020 Here's the list of autostyle changes that could qualify as breaking change enhancements:
https://github.com/autostyle/autostyle/blob/master/CHANGES.md
For example using a build cache.
Incremental build would be nice too@nedtwigg The more I work with this approach, the less I feel like it's going to work. In order to really move to use the latest Gradle APIs, we'll want to switch the tasks and extensions to use
Providerfor input/output properties. This way we can remove theafterEvaluatecall that wires extension values into the tasks.Instead of trying to do this in a single branch, what do you think of using 2 different Git branches and publishing the 2 plugins separately? There will be a bit more overhead in keeping the branches in sync, and we'd need a second CI pipeline, but I think we can avoid making nasty compromises in the modern implementation.
WDYT?
@LifeIsStrange, as noted in #616, we already support incremental build and the build cache.
@bigdaz I definitely don't want nasty compromises! How about if
SpotlessTaskModernandSpotlessPluginModernetc. don't extend the base classes at all? That should provide just as much flexibility as if they were in a new branch. Also, feel free to just@Ignoreany troublesome tests.I am absolutely killing the old implementation, but I would like to have them parallel in the same source tree. Before releasing, I will do the following transformation:
SpotlessTaskModern->SpotlessTaskSpotlessTask->SpotlessTaskLegacy
But it's easier for me to understand and maintain your changes if I can review and test them parallel to the legacy implementation. I don't mean to burden you with the testing part of it though, feel free to dump that part on me.
Reacted by LifeIsStrangeThanks Ned. I understand your motivation, and I'll keep working to do the minimal amount of work so that we can avoid creating tasks eagerly. Yes, and this might involve removing the shared base class for certain types.
At a later date you might want to change
FormatExtension,SpotlessExtensionand the various tasks to useProvider<X>for property types: this will allow us to get rid of theproject.afterEvaluatethat's required to ensure the extension is completely populated before reading, and will also permit more idiomatic wiring of task inputs/outputs. Right now, we really don't want to forkFormatExtensionfor SpotlessModern, since there are many subclasses.Feel free to break whatever you would like, including
FormatExtension, and I can put them back together / abandon the legacy as needed. My vague sense was that these:could become
List<Action<T>>in the modern side, which get evaluated one-by-one on task configuration, and that would allow us to remove theafterEvaluatewithout making changes toFormatExtension. I get the value ofTaskProvider<>, but I haven't yet gotten my head around the value ofProvider<>for inputs and outputs. But given how it's a big feature, I think it's a limitation of my understanding, so I'm open to whatever changes you think are best, back-compat be damned.
Currently, the plugin id for Spotless is
com.diffplug.gradle.spotlessand it is backward-compatible all the way to Gradle2.x. Soon, it will cohabitate withcom.diffplug.spotless, which will require Gradle5.4+, drops all deprecated code, and adopts all latest-available Gradle APIs. If your build completes without warnings on the old id, then all you'll need to do is swap the new id with no further changes.It will probably be some weeks/months before
com.diffplug.spotlessis ready. In the meantime, if you specify-PspotlessModern=true, then you'll be using the new plugin already. We are currently running our test suite using this flag, to ensure that the transition is seamless. (PR #598)The whole point of Spotless is that formatting doesn't matter, so it follows that code formatting tools really don't matter. This is a unique opportunity to drop confusing features, but anything that gets dropped needs to have a very clear migration path, or else we'll just keep supporting it at the new id, likely forever.