-
Notifications
You must be signed in to change notification settings - Fork 292
Update to Gradle 7 #1476
base: master
Are you sure you want to change the base?
Update to Gradle 7 #1476
Conversation
Having one settings.gradle in each subfolder is not needed. Also rewriting the rootProject name is not recommended.
Moving build script task creation to configuration or execution will improve performance and caching findByName is an older method replaced by named() which also does not trigger task creation during configuration.
Why: Dependencies were using the older configuration names 'compile' and 'testCompile' which are deprecated. Replacing them with 'implementaiton' caused a lot of compilation errors which made it required to refactor all dependencies. How: Adding dependency-analysis plugin and using the suggested changes in dependencies. This also included adding of all transitive dependencies as direct dependencies wherever code usage existed. Common dependencies were added as they were used in several sub projects. To keep them in sync, a version variable was added according to previous standard.
Also fix 2 deprecation warnings: Update reports according to api, replacing enabled with required Adding explicit task dependency when using generated sources as input. Third deprecation warning is due to usage inside the io.spring.dependency-management plugin and cannot be fixed until plugin is updated.
efe077b
to
316c381
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
forgot to submit
@@ -1,5 +1,5 @@ | |||
distributionBase=GRADLE_USER_HOME | |||
distributionPath=wrapper/dists | |||
distributionUrl=https\://services.gradle.org/distributions/gradle-6.8.3-bin.zip | |||
distributionUrl=https\://services.gradle.org/distributions/gradle-7.6-bin.zip |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done now
@@ -1 +0,0 @@ | |||
rootProject.name = 'acceptance-test' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why was this removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was not sure about that change, but the code seemed strange. I removed all of the settings.gradle files since there was already one in the root, and the overall speed of the project made it redundant to have every sub project on it's own. Perhaps it is wrong. We'll see.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i agree that it should be removed unless these projects can be buillt independently of the root project.
@@ -1,3 +1,7 @@ | |||
plugins { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did you mean to intentionally keep this plugin here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I did, I thought it was a good idea to keep the project clean for other maintainers.
Perhaps I should have added a small readme about it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or I could remove it again
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there are a large number of changes in this PR and it makes it hard to review. i would move this to a separate PR.
@@ -163,6 +174,11 @@ tasks.named("compileJava").configure { | |||
source(generateAvro) | |||
} | |||
|
|||
tasks.named("checkstyleMain").configure { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this a new task? or was this moved from somewhere else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
new configuration of the task (added from root project) to fix deprecation warning
(warning: no dependency from checkstyleMain towards compileTestJava)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah gotcha 👍
Separated into 5 commits:
Preparation is made in first 3 commits, minor cleanup plus improving performance by delaying/avoiding task creation.
Main upgrade work is done in last 2 commits. Dependency changes were major but required, and builds still runs fine.