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

Update dependency resolution management #2000

Merged

Conversation

adamszewe
Copy link
Contributor

@adamszewe adamszewe commented Nov 11, 2023

Changes

  • get rid of the JCenter repository
  • centralize repository declaration for all projects in project settings
  • refactor build.gradle and settings.gradle from Groovy syntax to Kotlin DSL (thus replacing those files with build.gradle.kts and settings.gradle.kts respectively)

The remaining build.gradle files are refactored in: #2022

@adamszewe adamszewe marked this pull request as ready for review November 12, 2023 12:03
@imsodin
Copy link
Member

imsodin commented Nov 28, 2023

What's the reason for switching the style/syntax? Not that I have any strong preferences, but the existing one I have already seen/used and the new one not at all, so I'd like to hear about some incentives for the switch :)

There are also a few other build.gradle files, and I'd like to have a single syntax/style everywhere.

imsodin pushed a commit that referenced this pull request Nov 30, 2023
## Description
Back in 2021 [JFrog
announced](https://jfrog.com/blog/into-the-sunset-bintray-jcenter-gocenter-and-chartcenter/)
that they are deprecating the JCenter repository. It's still read-only
but it shouldn't be depended upon.

I was trying to remove it in the other PR:
#2000, but then I
found out a single dependency still needs it:
[libsuperuser](https://github.com/Chainfire/libsuperuser).
The most "up to date" version [can be found in
JitPack](https://jitpack.io/#Chainfire/libsuperuser).
I had to update the version manually as dependabot would never pick it
up until JitPack is added to the list of repositories.

## Changes
* add the JitPack as a new repository to the project
* update the `libsuperuser` dependency to version `1.1.1`
@adamszewe
Copy link
Contributor Author

What's the reason for switching the style/syntax? Not that I have any strong preferences, but the existing one I have already seen/used and the new one not at all, so I'd like to hear about some incentives for the switch :)

There are also a few other build.gradle files, and I'd like to have a single syntax/style everywhere.

Those files were using Groovy language and they were refactored to use Kotlin DSL.
The main advantage of using Kotlin instead of Groovy is the IDE support. With Groovy it is extremely limited, since it is dynamically typed, while Kotlin is statically typed.
Kotlin is also the default language when it comes to developing Android applications, thus using the same language for both build logic and the app itself is also quite helpful.
See also: Kotlin DSL is Now the Default for New Gradle Builds.

I'd happily refactor the other remaining build.gradle files from Groovy to Kotlin 😄

@adamszewe adamszewe force-pushed the update-dependency-resolution-management branch from eb3608e to af2a919 Compare December 5, 2023 20:51
@imsodin
Copy link
Member

imsodin commented Dec 5, 2023

Thanks for explaining - this being the new default is particularly convincing to me :)

Please do change all of them, I'd like to use one style/DSL only throughout the project.

@adamszewe
Copy link
Contributor Author

I opened a new PR that completes this refactor: #2022
I preferred to open another one to make it easier to review.

imsodin pushed a commit that referenced this pull request Dec 28, 2023
## Description:
Refactor gradle build files to use Kotlin DSL instead of Groovy.
There were also a bash script and a python script that needed to be
updated because they relied on parsing the `build.gradle` files written
in Groovy.

This PR completes the work started in:
#2000
@adamszewe adamszewe force-pushed the update-dependency-resolution-management branch from af2a919 to 92b6ba8 Compare January 1, 2024 17:07
@adamszewe
Copy link
Contributor Author

@imsodin I rebased the PR to get latest updates and this PR is ready to review

Comment on lines 11 to 15
repositories {
gradlePluginPortal()
google()
jcenter()
mavenCentral()
}
Copy link
Member

Choose a reason for hiding this comment

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

Just to double check: This block needs to be present both here and in settings.gradle.kts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it still has to be present because the buildscript block is used.
If it's removed the build fails as it cannot resolve those dependencies in the buildscript.

image

It should be possible to get rid of this, I'll try to do that in a future PR.

Copy link
Member

Choose a reason for hiding this comment

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

Oh no worries, just wanted to make sure this wasn't a simple oversight

@imsodin imsodin merged commit 5c249ae into syncthing:main Jan 3, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants