github-workflows-kt icon indicating copy to clipboard operation
github-workflows-kt copied to clipboard

Update Gradle config to share common configuration

Open aSemy opened this issue 2 years ago • 10 comments

Hi, I thought I'd make a PR directly rather than an issue - I hope that's okay.

I noticed the Gradle build files had some repetition that could be tidied up by using buildSrc convention plugins.

I also tidied up some of the Gradle task definitions.

To help me work with the existing style I added a simple Editor Config file.

If accepted, here are some notes on how to develop in the future:

  • The versions of Gradle plugins are defined in one place: ./buildSrc/build.gradle.kts
  • To add a new Gradle plugin, add the Maven coordinates to ./buildSrc/build.gradle.kts
  • Repositories are defined in one place: ./buildSrc/repositories.settings.gradle.kts
  • To change a Kotlin setting for all subprojects, change it in the kotlin-jvm.gradle.kts buildSrc plugin. To change a setting for one subproject, change it in that subproject

The versions of dependencies could be defined in one place using the 'Java Platform' plugin and/or using a version catalog - I can implement this if you'd like!

aSemy avatar Apr 17 '22 10:04 aSemy

For dependencies, refreshVersions can take care of both generating the versions catalog and finding available updates instead of dependabot, but I don't know what @krzema12 thinks

jmfayard avatar Apr 17 '22 14:04 jmfayard

For dependencies, refreshVersions can take care of both generating the versions catalog and finding available updates instead of dependabot, but I don't know what @krzema12 thinks

@jmfayard to be honest, Dependabot does a pretty good job here. It creates PRs that I can review and merge from my phone, no need to actually change something in the code. As far as I see, refreshVersions takes a different approach by adding comments with available versions, which makes the process slightly more complicated. I know it's your library so please don't take it personally, I'm just pragmatically choosing an option that means less effort for me :)

krzema12 avatar Apr 20 '22 17:04 krzema12

Side-note: due to my chronical lack of time, such refactoring would be much easier for me to review if it tackled a smaller bit at once. E. g. extracting publishing or applying ktlint and detekt to all projects. Otherwise it's hard to follow which change relates to what kind of improvement.

krzema12 avatar Apr 28 '22 06:04 krzema12

FYI I started applying this PR in smalller chunks (some smaller changes already merged), but I'm struggling to have a smooth experience with IntelliJ and these local convention plugins. I faced some issue with multiple Kotlin compiler artifacts where Gradle refused to proceed, it's also not trivial to get what Kotlin versions are safe to use where. It got me thinking if we really want to have these convention plugins. I'll give it another shot later.

krzema12 avatar Jun 17 '22 12:06 krzema12

FYI I started applying this PR in smalller chunks (some smaller changes already merged), but I'm struggling to have a smooth experience with IntelliJ and these local convention plugins. I faced some issue with multiple Kotlin compiler artifacts where Gradle refused to proceed, it's also not trivial to get what Kotlin versions are safe to use where. It got me thinking if we really want to have these convention plugins. I'll give it another shot later.

Great! It's really good to hear. Let me know if there's anything else.

Yes the experience can be really tough. I have lots of trouble because in buildSrc IntelliJ flags lots of code Gradle Kotlin DSL shortcuts as invalid, even though it's not. It makes it hard to figure out what's right and what's wrong.

The Gradle/Kotlin compatibility is documented here: https://docs.gradle.org/current/userguide/compatibility.html#kotlin. Currently Gradle will set the Kotlin language level to 1.4, which it is quite strict about. Gradle uses an embedded version of Kotlin, which is 1.5.x at the moment - but you can safely use 1.6.21 in ./buildSrc/build.gradle.kts

In Gradle 7.5 the embedded version of Kotlin will be bumped to 1.6.21, but the language level will still be set to 1.4 - it's annoying how Gradle is lagging behind.

aSemy avatar Jun 18 '22 10:06 aSemy

https://github.com/dependabot/dependabot-core/issues/2180

Is it true that after this change we won't have Dependabot updates? It's a major disadvantage to me 😢

krzema12 avatar Jul 03 '22 17:07 krzema12

dependabot/dependabot-core#2180

Is it true that after this change we won't have Dependabot updates? It's a major disadvantage to me 😢

Hmm maybe. Does Dependabot recommend updates for Kotlin, Detekt, and ktlint Gradle plugins? Because those are the only versions that are defined within buildSrc: https://github.com/krzema12/github-actions-kotlin-dsl/pull/194/files#diff-7b5c40eb5522b5216cfe8c75612e579a8db0daee9ded80df9ce6a56e8361a962

The versions of dependencies are still set in the regular subproject build.gradle.kts files, like before. But I don't know how Dependabot works with that. I've not used Dependabot before, but from reading the issues it seems like Dependabot has never really fully supported Gradle. Apparently Renovate does?

Personally I like using version catalogs (all the dependencies and versions are defined in a single file, libs.versions.toml), so I only have to check versions in single place, and use a version platform (Gradle java-platform plugin) to keep versions (including transitive dependencies) aligned. Hopefully Dependabot supports that soon https://github.com/dependabot/dependabot-core/issues/3121

aSemy avatar Jul 03 '22 18:07 aSemy

Yes, Dependabot also handles plugin versions. Example: https://github.com/krzema12/github-actions-kotlin-dsl/commit/5f1d98754e9c77cce65f8e0896ed15fdc205bbeb

Great, thanks, I'll give Renovate and version catalog a shot!

krzema12 avatar Jul 03 '22 18:07 krzema12

@aSemy would you be so kind to rebase these changes on newest main branch? Most of the minor ones should be already applied, and I'm most interested in using the precompiled plugins. Renovate seems to support these. Regarding version catalog, let's hold with it for now, we may try it in a separate PR. In this one I'd like to focus only on extracting stuff to precompiled plugins.

krzema12 avatar Aug 01 '22 22:08 krzema12

@aSemy would you be so kind to rebase these changes on newest main branch? Most of the minor ones should be already applied, and I'm most interested in using the precompiled plugins. Renovate seems to support these. Regarding version catalog, let's hold with it for now, we may try it in a separate PR. In this one I'd like to focus only on extracting stuff to precompiled plugins.

Done!

I refreshed some of the implementations based on some Gradle things I've learned.

I also created a separate task type for that 'check Maven Central' script task, as a bit of a demo to show how to do that.

I didn't test this significantly, so some things might be janky, or maybe I didn't properly refresh the versions after the merge

aSemy avatar Aug 02 '22 20:08 aSemy

In https://github.com/krzema12/github-actions-kotlin-dsl/pull/386 I'm merging a part of this PR - just the kotlin-jvm plugin, centralized repos management plus my little tweaks, to see how they work in practice and to not touch releasing config for now. If everything goes fine and e.g. Renovate works fine in this configuration, I'll proceed with introducing changes in this PR :smile:

krzema12 avatar Aug 05 '22 21:08 krzema12

Renovate works fine for the precompiled plugins: https://github.com/krzema12/github-actions-kotlin-dsl/commit/1832f83159d5b4896e313610312e342dbba8525a. I'll merge the rest of this PR soon-ish 😄

krzema12 avatar Aug 10 '22 21:08 krzema12

It took me a while to understand and merge this PR (in several pieces), but we finally did it! :muscle: Thanks a lot for sharing your knowledge about Gradle and answering all my question in detail. I just became a more conscious user of it :wink: Plus the code base got cleaner. I'm open to any future improvements, but please let's make them in smaller, easier to digest pieces. Best if one thing is changed at a time - no confusion :arrow_right: fast merge :smiley:

Cheers!

krzema12 avatar Aug 14 '22 22:08 krzema12