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

Todo list if/when we drop compat for an older Gradle version #504

Closed
3 tasks done
nedtwigg opened this issue Jan 1, 2020 · 15 comments
Closed
3 tasks done

Todo list if/when we drop compat for an older Gradle version #504

nedtwigg opened this issue Jan 1, 2020 · 15 comments

Comments

@nedtwigg
Copy link
Member

nedtwigg commented Jan 1, 2020

To my knowledge, the only thing we're sacrificing by maintaining support for 2.14 is task configuration avoidance. We do this internally already, so we don't have much to gain from adopting it. But I figured I'd make a central place to keep track of where we're not using the latest APIs, and other changes we can make when/if we cut ties with older versions.

@nedtwigg
Copy link
Member Author

nedtwigg commented Feb 25, 2020

  • Rename which GradleIntegrationTest (and the like) to GradleIntegrationTestHarness. As per @t-rad679, it's best practice for the names only of test classes to end in Test.

@JLLeitschuh
Copy link
Member

Aren't you still coupled to 2.14 for some internal projects you own?

@nedtwigg
Copy link
Member Author

Not anymore. However, I still don't see a reason to drop compat until Gradle starts throwing deprecation warnings for something that we are using which is required for 2.14. We just had a deep and long-standing bug get fixed by somebody who is stuck on Gradle 3.x

@nedtwigg
Copy link
Member Author

nedtwigg commented Apr 2, 2020

The buildcache was announced in 3.5, but the @CacheableTask annotation exists all the way back to 3.0.

@nedtwigg
Copy link
Member Author

nedtwigg commented May 2, 2020

The just-released google-java-format 1.8 requires Java 11 (#563). Eclipse 4.17 (Sep 2020) is going to require Java 11 as well (#547). Gradle 5.0 is the first version of Gradle which supports running on Java 11.

We normally keep the default versions of formatters set to their latest available versions (users can override), but now we're going to have to keep the defaults back for as long as we support Java 8.

@nedtwigg
Copy link
Member Author

nedtwigg commented May 9, 2020

As of now, the deprecated IncrementalTaskInputs that we use is scheduled to begin nagging in Gradle 7 - gradle/gradle#9048. Fixing that would bump our minimum required Gradle to 5.4.

I'm hoping we can get #280 and #511 resolved before that so they're available to people who are stuck on older Gradle for whatever reason.

@nedtwigg
Copy link
Member Author

nedtwigg commented May 13, 2020

I propose the following roadmap:

The "goals" in each phase are loose. One of my top priorities personally is to not be a blocker on other people's work, so if a good PR ready to ship then we'll ship it, major versions be damned. But I'd like to get #511 out and available all the way back to 2.x. IMO, it removes every single excuse to not use an autoformatter, no matter what version of Gradle you might be stuck on.

I also want to minimize the if gradleOld then blah since we need to bump to 5.4 very soon anyway. So I'm asking for a stay until June 1st at the latest. Once June 1st comes around, we'll set Gradle 5.4 as the minimum supported, and it'll be open season to adopt all of Gradle's latest and greatest. If I can get #511 merged sooner, then we'll bump minimum-required to 5.4 sooner. I've got a prototype for #511 on my box, just wanted to get #576 merged first since it touches more things.

@bigdaz
Copy link
Contributor

bigdaz commented May 13, 2020

The overall roadmap sounds great, Ned.

From my point of view, I'd be fine if the changes to make Spotless cacheable were all deferred until "ratchet" support was added and released for all versions of Gradle. I would be content to rework my PR to use InputChanges and rebase on top of whatever changes come are required to support ratchet. (In fact, the #576 implementation may be cleaner with InputChanges, due to the availability of FileChange.getNormalizedPath())

I don't feel strongly about it, but I assume that deferring #576 (and related PRs) could avoid requiring 2 separate major releases. Since we're only talking about a few weeks delay, I don't see much downside.

@nedtwigg
Copy link
Member Author

Thanks! The other thing I failed to mention is that #576 makes #511 a lot easier! Part of why #511 is still sitting in a local stash is because of up-to-date challenges which you have fixed for me :) So I definitely want to merge and release #576 now.

I don’t mind bumping major version when the cost to our users is low or the benefit to them is high, and the cost of the 4.x bump incurred by #576 is very low. I’ve probably seemed like a stability-maximalist to people who have wanted us to adopt newer versions of Gradle, but I’m all for bumping major version when (benefit/cost) >> 1

@bigdaz
Copy link
Contributor

bigdaz commented May 13, 2020

Right, that makes sense then. Glad that my changes make developing other features easier.
I certainly think the new pattern is easier to extend, and easier for my brain to process :).

@nedtwigg
Copy link
Member Author

@jbduncan @fvgh do you have any objections to the roadmap above? I've got 4.0 ready to ship, but I wanted to make sure that you two are okay with that plan before I take any irreversible steps in that direction.

@jbduncan
Copy link
Member

@nedtwigg No objections regarding the roadmap! If I come across a roadblock upgrading anything from Spotless 3.x to 4-5.x, I'll raise a new issue and let you know. :)

@nedtwigg
Copy link
Member Author

nedtwigg commented Jun 2, 2020

Small change to the roadmap. During the 4.x train, we will ship com.diffplug.gradle.spotless (gradle 2.x+) and an incubating version of com.diffplug.spotless (gradle 5.4+) in the same jar. I've got a PoC branch which lets us run our existing test suite against both, so that we can be confident that we're not breaking anything on accident. I expect to get that up today.

The change to com.diffplug.spotless is an especially good time to look at how we define target. It's grown somewhat complex, and there have been some valid issues raised by people, which we have closed as wontfix due to backcompat. While both plugins live in the same jar, we have a unique opportunity to do something like spotless5xMigrationCheck, where we could change the target behavior, and allow users to confirm that they have adjusted their project to format all the same files as before.

@nedtwigg
Copy link
Member Author

nedtwigg commented Jun 5, 2020

With #598 merged, we're ready to start adopting all the latest Gradle APIs, and we're also ready to make sure users have a smooth migration. Since the todos are closed out, the new "modern" plugin will be tracked in a new issue, #600

@nedtwigg nedtwigg closed this as completed Jun 5, 2020
@nedtwigg
Copy link
Member Author

nedtwigg commented Jul 2, 2020

Just wanted to drop you a line @fvgh and @jbduncan. I just released lib 2.0.0 and plugin-maven 2.0.0. The next project will be to strip all the deprecated code from plugin-gradle. I did a rehash of our docs, which exposed some inconsistencies, which led me to believe that we should cut some small things (namely default targets, but some other small things too).

If there's anything that I cut, which you would like to be put back, I'm 100% open to putting it back! Since we had to take a breaking change in order to fix some caching issues, and we can always add them back without a breaking change, I leaned towards "cut it" for things where I was on the fence.

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

No branches or pull requests

4 participants