Skip to content
This repository was archived by the owner on Jun 7, 2024. It is now read-only.

Update to Gradle 7#1476

Open
Jocce-Nilsson wants to merge 5 commits intozalando:masterfrom
Jocce-Nilsson:master
Open

Update to Gradle 7#1476
Jocce-Nilsson wants to merge 5 commits intozalando:masterfrom
Jocce-Nilsson:master

Conversation

@Jocce-Nilsson
Copy link
Copy Markdown

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.

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.
Comment thread gradle/wrapper/gradle-wrapper.properties
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.
Copy link
Copy Markdown

@runningcode runningcode left a comment

Choose a reason for hiding this comment

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

forgot to submit

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.

Copy link
Copy Markdown
Author

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'
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

why was this removed?

Copy link
Copy Markdown
Author

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.

Copy link
Copy Markdown

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.

Comment thread build.gradle
@@ -1,3 +1,7 @@
plugins {
Copy link
Copy Markdown

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?

Copy link
Copy Markdown
Author

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?

Copy link
Copy Markdown
Author

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

Copy link
Copy Markdown

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.

Comment thread core-common/build.gradle
source(generateAvro)
}

tasks.named("checkstyleMain").configure {
Copy link
Copy Markdown

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?

Copy link
Copy Markdown
Author

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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ah gotcha 👍

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants