-
Notifications
You must be signed in to change notification settings - Fork 594
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
Upgrade to gradle 5.4.1 #6007
Upgrade to gradle 5.4.1 #6007
Conversation
Codecov Report
@@ Coverage Diff @@
## master #6007 +/- ##
================================================
- Coverage 86.931% 35.189% -51.742%
+ Complexity 32754 17310 -15444
================================================
Files 2016 2011 -5
Lines 151426 150912 -514
Branches 16623 16131 -492
================================================
- Hits 131636 53105 -78531
- Misses 13728 92939 +79211
+ Partials 6062 4868 -1194
|
@lbergelson are you able to help with the docker/gradle failures? Also, there are likely more gradle changes needed for publishing artifacts, but I'm not sure what's needed or how to test those changes. |
task wrapper(type: Wrapper) { | ||
gradleVersion = '3.1' | ||
wrapper { | ||
gradleVersion = '5.4.1' | ||
} |
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.
@tomwhite You need to update dockertest.gradle
as well. The needed change is in my PR here: https://github.com/broadinstitute/gatk/pull/5928/files#diff-20d46fff5956131a01d67961f7ec15c0
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.
Thanks @droazen, that fixes it. All the tests now pass. Are there any other changes needed before merging?
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.
@lbergelson suggested that we test the maven artifact uploading, as he suspects it might break in this PR. We'll test that soon and get back to you on the results.
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 fixed a deprecation warning and rebuild the wrapper itself. Haven't changed the publishing over yet.
I fixed the artifact uploading as well now. Everything should be good provided tests are passing. It turned out to be something really simple and dumb. |
We are two major versions behind the latest gradle release. A later version is needed for upgrading Spark (#5990) and using BigQuery (#5928).