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

Update Gradle to 7.4.1 #2078

Merged
merged 2 commits into from
Mar 29, 2022
Merged

Update Gradle to 7.4.1 #2078

merged 2 commits into from
Mar 29, 2022

Conversation

reta
Copy link
Collaborator

@reta reta commented Feb 9, 2022

Signed-off-by: Andriy Redko andriy.redko@aiven.io

Description

Update Gradle to 7.4 (never ending story)
Release notes: https://docs.gradle.org/7.4/release-notes.html

Issues Resolved

N/A

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@reta reta requested a review from a team as a code owner February 9, 2022 14:58
@opensearch-ci-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure a0be7fd535c0ec094b771c445e53337480c0c853
Log 2308

Reports 2308

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure fef1ef5b33b51f54d12deb9fd59c71e1f338af85
Log 2310

Reports 2310

@reta reta marked this pull request as draft February 9, 2022 17:43
@reta reta changed the title Update Gradle to 7.4 [DRAFT] Update Gradle to 7.4 Feb 9, 2022
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 591b75fd52d906f7c51f4bc4de6c89e097e3841c
Log 2311

Reports 2311

@reta
Copy link
Collaborator Author

reta commented Feb 9, 2022

Converting to draft, needs [1]. [2] to be fixed first [3]

[1] https://github.com/policeman-tools/forbidden-apis
[2] https://github.com/policeman-tools/forbidden-apis/blob/main/src/main/java/de/thetaphi/forbiddenapis/gradle/CheckForbiddenApis.java#L467
[3] policeman-tools/forbidden-apis#189

@reta
Copy link
Collaborator Author

reta commented Feb 9, 2022

Thanks for help @uschindler !

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 0b85091ffd683dfbd420d088f53488b1e54b8dc4
Log 2312

Reports 2312

@uschindler uschindler mentioned this pull request Feb 9, 2022
Closed
@reta reta changed the title [DRAFT] Update Gradle to 7.4 [DRAFT] Update Gradle to 7.4.1 Mar 16, 2022
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure de9d8343643bf9497ccefcba08e3f1b44f623eea
Log 3456

Reports 3456

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 4ed8886fb0e3a1450ff64b58707193bf306926a6
Log 3458

Reports 3458

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 87851c17464ebf1ea95dae79678cb8f5669269b1
Log 3487

Reports 3487

@reta
Update Gradle to 7.4.1
4815eff 
Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
@reta reta changed the title [DRAFT] Update Gradle to 7.4.1 Update Gradle to 7.4.1 Mar 28, 2022
@reta reta marked this pull request as ready for review March 28, 2022 12:46
uschindler
uschindler approved these changes Mar 28, 2022
View reviewed changes
Copy link
Contributor

@uschindler uschindler left a comment

Choose a reason for hiding this comment

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

Looks fine.

@uschindler
Copy link
Contributor

I think, you may add @PathSensitive(PathSensitivity.RELATIVE) to the other tasks, too. I think none of them relies on absolute paths.

@reta
Copy link
Collaborator Author

reta commented Mar 28, 2022

x Gradle Check failure 4815eff Log 3787

Reports 3787

#2538 :(

@reta
Copy link
Collaborator Author

reta commented Mar 28, 2022

I think, you may add @PathSensitive(PathSensitivity.RELATIVE) to the other tasks, too. I think none of them relies on absolute paths.

Added everywhere except `ThirdPartyAuditTask.java, it scans JARs which should not be relative to the project (imho), thanks @uschindler !

@uschindler
Copy link
Contributor

I think, you may add @PathSensitive(PathSensitivity.RELATIVE) to the other tasks, too. I think none of them relies on absolute paths.

Added everywhere except `ThirdPartyAuditTask.java, it scans JARs which should not be relative to the project (imho), thanks @uschindler !

I do not know what the default is. Maybe check and be explicit for the one with JAR files. But this is not fully related to this task PR.

@reta
Copy link
Collaborator Author

reta commented Mar 28, 2022

I think, you may add @PathSensitive(PathSensitivity.RELATIVE) to the other tasks, too. I think none of them relies on absolute paths.

Added everywhere except `ThirdPartyAuditTask.java, it scans JARs which should not be relative to the project (imho), thanks @uschindler !

I do not know what the default is. Maybe check and be explicit for the one with JAR files. But this is not fully related to this task PR.

As per official docs, https://docs.gradle.org/current/javadoc/org/gradle/api/tasks/PathSensitive.html:

If a Task declares a file property without this annotation, the default is PathSensitivity.ABSOLUTE.

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 0f14f2026933613d99b7a3236d78a183734fe5b3
Log 3804

Reports 3804

@reta
Copy link
Collaborator Author

reta commented Mar 28, 2022

x Gradle Check failure 0f14f20 Log 3804

Reports 3804

Jackpot #2242 & #1561

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success ff27115ae11e90a1580f4049002c63fec510ace0
Log 3805

Reports 3805

@reta
Copy link
Collaborator Author

reta commented Mar 28, 2022

@andrross @dblock could you please take a look guys? thank you!

andrross
andrross reviewed Mar 28, 2022
View reviewed changes
Copy link
Member

@andrross andrross left a comment

Choose a reason for hiding this comment

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

Just for my own edification, did you run ./gradlew wrapper --gradle-version=7.4.1 to do the update? When I did that, it also made a lot of changes to gradlew but I don't see that file changed in this PR.

gradle/wrapper/gradle-wrapper.properties Outdated Show resolved Hide resolved
@reta
Address code review comments, added @PathSensitive(PathSensitivity.RE…
54e95a0 
…LATIVE) where applicable

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 54e95a0
Log 3847

Reports 3847

@dblock dblock merged commit 223efe6 into opensearch-project:main Mar 29, 2022
@dblock dblock added the backport 2.x Backport to 2.x branch label Mar 29, 2022
@dblock
Copy link
Member

dblock commented Mar 29, 2022

I am going to skip the backport to 2.0 to keep that stable, but let me know if you feel otherwise.

opensearch-trigger-bot bot pushed a commit that referenced this pull request Mar 29, 2022
@reta @github-actions
Update Gradle to 7.4.1 (#2078)
c33401b 
* Update Gradle to 7.4.1

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

* Address code review comments, added @PathSensitive(PathSensitivity.RELATIVE) where applicable

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
(cherry picked from commit 223efe6)
@opensearch-trigger-bot opensearch-trigger-bot bot mentioned this pull request Mar 29, 2022
Merged
@reta
Copy link
Collaborator Author

reta commented Mar 29, 2022

I am going to skip the backport to 2.0 to keep that stable, but let me know if you feel otherwise.

I think it is low risk change, would be good to have it in 2.0, @andrross wdyt?

@dblock dblock added the backport 2.0 Backport to 2.0 branch label Mar 29, 2022
@dblock
Copy link
Member

dblock commented Mar 29, 2022

@CEHENKLE WDYT?

opensearch-trigger-bot bot pushed a commit that referenced this pull request Mar 29, 2022
@reta @github-actions
Update Gradle to 7.4.1 (#2078)
4cdc80c 
* Update Gradle to 7.4.1

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

* Address code review comments, added @PathSensitive(PathSensitivity.RELATIVE) where applicable

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
(cherry picked from commit 223efe6)
@opensearch-trigger-bot opensearch-trigger-bot bot mentioned this pull request Mar 29, 2022
Closed
@tlfeng
Copy link
Collaborator

tlfeng commented Mar 29, 2022

Thank you for making this upgrade @reta !
I saw an advantage of Gradle 7.4 is making the code coverage report generation easier for this OpenSearch repository. (https://docs.gradle.org/7.4/release-notes.html#aggregation-tests).
Probably the current script of creating the code coverage report (https://github.com/opensearch-project/OpenSearch/blob/1.3.0/gradle/code-coverage.gradle) can be simplified a lot. 😄

@reta
Copy link
Collaborator Author

reta commented Mar 29, 2022

Thank @tlfeng , yes, aggregate coverage reports is very appealing, we have to explore that, I agree

dblock pushed a commit that referenced this pull request Mar 30, 2022
@opensearch-trigger-bot @reta
Update Gradle to 7.4.1 (#2078) (#2645)
8865e75 
* Update Gradle to 7.4.1

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

* Address code review comments, added @PathSensitive(PathSensitivity.RELATIVE) where applicable

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
(cherry picked from commit 223efe6)

Co-authored-by: Andriy Redko <andriy.redko@aiven.io>
@dblock dblock mentioned this pull request Apr 14, 2022
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrross andrross andrross left review comments

@uschindler uschindler uschindler approved these changes

@dblock dblock dblock approved these changes

Assignees
No one assigned
Labels
backport 2.x Backport to 2.x branch backport 2.0 Backport to 2.0 branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@reta @opensearch-ci-bot @uschindler @dblock @tlfeng @andrross