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

Improve Gradle-related Kotlin code #1004

Closed
17 tasks done
MaciejG604 opened this issue Jul 26, 2022 · 5 comments · Fixed by #1006, #1007, #1012, #1014 or #1015
Closed
17 tasks done

Improve Gradle-related Kotlin code #1004

MaciejG604 opened this issue Jul 26, 2022 · 5 comments · Fixed by #1006, #1007, #1012, #1014 or #1015
Assignees
Labels
Milestone

Comments

@MaciejG604
Copy link
Contributor

MaciejG604 commented Jul 26, 2022

After migrating the gradle scripts to Kotlin there are some issues that need to be resolved:

  • Find a way to use version catalog aliases in helpers' code in buildSrc/
    (one way to do it would be to put the aliases into extra properties of the project in buildSrc)
  • Unify plugins' notations in libraries and plugins in libs.version.toml:
    • the versions shared with the [plugins] sections should be extracted to [versions] section to avoid discrepancies
    • or maybe the [plugins] section can be removed completely in favor of the newly-added [libraries]?
    • extra care should be taken to retain @pins on certain plugins, as done in [plugins] currently (so that they're NOT auto-updated by gradle versionCatalogUpdate)
    • a comment would be useful why plugins in [libraries] are needed in the first place and/or a reference to a blogpost describing this technique
  • Remove code that's marked as Deprecated from CheckerFrameworkConfigurator::applySubtypingChecker, then revert kotlinOptions.allWarningsAsErrors = true in buildSrc/build.gradle.kts
  • Refactor IntellijVersionHelper so that there are no unsafe typecasts: preferably extract a data class to keep the IntelliJ versions retrieved from properties file, to avoid using string keys
  • Switch all classes in buildSrc to Kotlin
  • Remove all remaining support for Groovy from Gradle code (mostly Spotless)
  • Check if it's possible to keep build.gradle.kts files formatted with Spotless (and not just files in buildSrc/) — see kotlinGradle { ... }
  • Fix spotless for Kotlin so that it launches automatically with ./gradlew build
  • Fix Gradle warning in :buildSrc:compileKotlin task: 'compileJava' task (current target is 11) and 'compileKotlin' task (current target is 1.8) jvm target compatibility should be set to the same Java version
  • Check if there is any built-in way in Kotlin to use static identifiers for projects: something like api(project(branchLayout.api)). Probably no point in defining them explicitly if they aren't available OOTB => NO
  • Check if it's possible to replace BuildUtils.foo(project) calls with just foo() (probably an extension method on Project would be needed) => YES
  • Rewrite the latter half of UpdateEapBuildNumber#checkForEapWithBuildNumberHigherThan to use Kotlin regex API
  • Consider un-hardcoding names of IntellijVersions properties by using .name: https://kotlinlang.org/docs/reflection.html#property-references
@MaciejG604 MaciejG604 added the code quality Code quality label Jul 26, 2022
@MaciejG604 MaciejG604 added this to the v2.2.0 milestone Jul 26, 2022
@PawelLipski PawelLipski changed the title Improve gradle realted kotlin code Improve Gradle-related Kotlin code Jul 26, 2022
@PawelLipski
Copy link
Collaborator

(@MaciejG604 checkboxes FTW ☝🏻 )

@PawelLipski
Copy link
Collaborator

@MaciejG604 pls tick off once you start work on the given point. This will prevent doubling the efforts in case someone else would like to pick a subtask.

@MaciejG604
Copy link
Contributor Author

MaciejG604 commented Aug 5, 2022

I don't know if un-hardcoding names of IntellijVersions properties can be done while still keeping all the type guarantees. It is so becuase the class stores not only Strings (nullable and not-nullable) but also a List of Strings.
It was not hardcoded before using a Map<String, Any>, but the types could not be checked by the compiler.

@PawelLipski
Copy link
Collaborator

Ouch, sorry... I pointed to the wrong class. I meant unhardcoding keys in the dict used in com.virtuslab.gitmachete.buildsrc.IntellijVersionHelper#resolveIntelliJVersions ‼️

@MaciejG604
Copy link
Contributor Author

Also related but can't link as there's a limit of 10 PRs linked:
#1035, #1038

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