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.

Sign up for GitHub

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

Jump to bottom

Refactor/1004 simplify libs versions #1026

Merged
merged 7 commits into from
Aug 3, 2022
Merged

Refactor/1004 simplify libs versions #1026

merged 7 commits into from
Aug 3, 2022

Conversation

MaciejG604
Copy link
Contributor

No description provided.

@MaciejG604 MaciejG604 requested a review from PawelLipski as a code owner August 1, 2022 11:15
@MaciejG604
Copy link
Contributor Author

MaciejG604 commented Aug 1, 2022 •
edited
Loading

Concerning the removal of lines from [plugins] section we could do it in two ways:

  1. If the plugin is used only as a dependency to a precompiled script then we can remove it from [plugins] section, this way we still have both sections just cleaner, with less duplications
  2. We could entirely remove [plugins] section and use the legacy plugin application everywhere Gradle docs

@PawelLipski PawelLipski force-pushed the refactor/1004-deprecated branch from f53571d to 41162a5 Compare August 1, 2022 12:15
@PawelLipski
Copy link
Collaborator

If the plugin is used only as a dependency to a precompiled script then we can remove it from [plugins] section, this way we still have both sections just cleaner, with less duplications

Okay... do we have any such case now? any plugin that can be removed from [plugins] section?

We could entirely remove [plugins] section and use the legacy plugin application everywhere Gradle docs

Hmmm what would be downsides of such approach? will we still have e.g. the same IDE support? also, isn't legacy plugin application scheduled for removal soon-ish?

@PawelLipski
Copy link
Collaborator

Also, pls take care to provide proper PR name + milestone + labels + issue binding once you open a non-draft PR

@MaciejG604 MaciejG604 force-pushed the refactor/1004-simplify-libs-versions branch from 5c80697 to d27304b Compare August 1, 2022 12:35
Base automatically changed from refactor/1004-deprecated to develop August 1, 2022 13:55
@MaciejG604 MaciejG604 force-pushed the refactor/1004-simplify-libs-versions branch from d27304b to cc769d4 Compare August 1, 2022 14:05
@MaciejG604 MaciejG604 linked an issue Aug 1, 2022 that may be closed by this pull request
Improve Gradle-related Kotlin code #1004
Closed
17 tasks
@MaciejG604 MaciejG604 added this to the v2.2.0 milestone Aug 1, 2022
@MaciejG604 MaciejG604 self-assigned this Aug 1, 2022
@MaciejG604
Copy link
Contributor Author

The plugins that are now only used as dependencies are:

  • aspectj-postCompileWeaving
  • checkerFramework
  • jetbrains-intellij
  • spotless

That leaves us with only jetbrains-kotlin used both as a dependecy and being applied as a plugin.

@MaciejG604 MaciejG604 force-pushed the refactor/1004-simplify-libs-versions branch from c9cf728 to f8273c8 Compare August 2, 2022 12:04
PawelLipski
PawelLipski reviewed Aug 2, 2022
View reviewed changes
build.gradle.kts Outdated Show resolved Hide resolved
PawelLipski
PawelLipski reviewed Aug 2, 2022
View reviewed changes
gradle/libs.versions.toml
# @pin because as per https://plugins.jetbrains.com/docs/intellij/kotlin.html#kotlin-standard-library:
# "If a plugin supports multiple platform versions, it must (...) target the lowest bundled stdlib version"
# which is 1.4.0 in our case (as this is the Kotlin version bundled into IntelliJ 2020.3).
pluginPackages-jetbrains-kotlin = "org.jetbrains.kotlin:kotlin-gradle-plugin:1.4.0"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Huh AFAICS no warning on different Kotlin versions from buildSrc:compileKotlin... but also no reference to Gradle 7.4.2-compatible Kotlin 1.5. What's the trick then? 🤔

PawelLipski
PawelLipski reviewed Aug 2, 2022
View reviewed changes
gradle/libs.versions.toml
pluginPackages-spotless = { module = "com.diffplug.spotless:spotless-plugin-gradle", version.ref = "plugins-spotless" }
# @pin because 6.5.x is incompatible with kotlin plugin 1.4.x,
# see https://github.com/freefair/gradle-plugins/issues/554#issuecomment-1171434185 for more details
pluginPackages-aspectj-postCompileWeaving = "io.freefair.gradle:aspectj-plugin:6.4.3.1"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Try if it's possible to introduce some TOML subsection like [pluginPackages] or [libraries.pluginPackages] instead of prefixing each entry with pluginPackages-

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's not, haven't seen that in any docs, and the compiler throws this when trying to introduce [pluginPackages]:
Problem: In version catalog libs, unknown top level elements [pluginPackages]

@MaciejG604 MaciejG604 merged commit dead3f9 into develop Aug 3, 2022
@MaciejG604 MaciejG604 deleted the refactor/1004-simplify-libs-versions branch August 3, 2022 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PawelLipski PawelLipski PawelLipski approved these changes

Assignees

@MaciejG604 MaciejG604

Labels
build code quality Code quality
Projects
None yet
Milestone
v2.2.0
Development

Successfully merging this pull request may close these issues.

Improve Gradle-related Kotlin code
2 participants
@MaciejG604 @PawelLipski