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

crowdin-cli: Update subdependencies and code for Gradle 7.1 #371

Merged
merged 1 commit into from
Jun 30, 2021

Conversation

Myself5
Copy link
Contributor

@Myself5 Myself5 commented Jun 27, 2021

Gradle 6.5.1 is not working with Java 16 (and I couldn't get it working with Java 8 on ArchLinux either).

Changes:

  • Update maven URL to use HTTPS (Gradle 7 requirement).
  • Update Gradle and wrapper to the latest Version (7.1.1).
  • Update build.gradle to replace deprecated "enabled" flag with "required".
  • Update all subdependencies to latest as some required updates for the Gradle update.
  • Adjust code to match changes between zip4j 1.3.2 and 2.8.0.

Tests: Build, Pass tests, run a quick translation pull.

@codecov
Copy link

codecov bot commented Jun 27, 2021

Codecov Report

Merging #371 (ecbb381) into cli3 (7f74275) will not change coverage.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##               cli3     #371   +/-   ##
=========================================
  Coverage     58.13%   58.13%           
  Complexity      877      877           
=========================================
  Files           143      143           
  Lines          4186     4186           
  Branches        624      624           
=========================================
  Hits           2433     2433           
  Misses         1457     1457           
  Partials        296      296           
Impacted Files Coverage Δ
...om/crowdin/cli/commands/functionality/FsFiles.java 3.04% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7f74275...ecbb381. Read the comment docs.

@Myself5
Copy link
Contributor Author

Myself5 commented Jun 27, 2021

-1, still having some runtime issues. will solve ASAP

Gradle 6.5.1 is not working with Java 16 (and I couldn't get it working with Java 8 on ArchLinux either).

Changes:
* Update maven URL to use HTTPS (Gradle 7 requirement).
* Update Gradle and wrapper to the latest Version (7.1.1).
* Update build.gradle to replace deprecated "enabled" flag with "required".
* Update all subdependencies to latest as some required updates for the Gradle update.
* Adjust code to match changes between zip4j 1.3.2 and 2.8.0.

Tests: Build, Pass tests, run a quick translation pull.
@Myself5 Myself5 force-pushed the gradle-7.1-update branch from 35286cf to ecbb381 Compare June 28, 2021 12:55
@Myself5
Copy link
Contributor Author

Myself5 commented Jun 28, 2021

Further updated shadow, spotbugs, jacoco and checkstyle as those were incompatible with Java 16 as well. Working fine on my ArchLinux setup now.

@andrii-bodnar
Copy link
Member

@Myself5 thanks a lot for your contribution!

@andrii-bodnar andrii-bodnar linked an issue Jun 29, 2021 that may be closed by this pull request
@andrii-bodnar
Copy link
Member

@frombetelgeuse please review this PR.

@andrii-bodnar
Copy link
Member

@Myself5 will it be compatible with previous Gradle and java versions?

@Myself5
Copy link
Contributor Author

Myself5 commented Jun 29, 2021

Compatible with previous Gradle

Should be irrelevant, as the included gradle wrapper (which is 7.1) is what downloads Gradle and manages the rest. The "host" version of Gradle should therefore be irrelevant. (e.g. I don't even have gradle installed.)

Older Java Versions:

I've tested it with Java 16 and 8, Compiled it on 16, ran fine on 16 as well as 8, compiled it on 8, ran fine on 16 and 8 as well.

Both on a x86_64 system.

If there's any other edgecases that might need to be covered, let me know and I'll try.

@andrii-bodnar andrii-bodnar merged commit 3892207 into crowdin:cli3 Jun 30, 2021
@andrii-bodnar
Copy link
Member

@Myself5 merged this PR and discovered that the static analysis job was failed.

is there any way to fix this?

@Myself5
Copy link
Contributor Author

Myself5 commented Jun 30, 2021

Terribly sorry, I could swear that passed when I tested it. I'll take a look immediately.

@Myself5
Copy link
Contributor Author

Myself5 commented Jun 30, 2021

Ok, found it. The Upgrade to Checkstyle 8.44 appears to require some config updates, it's working fine with 8.34. Will check on which changes are required and open another PR.

@Myself5
Copy link
Contributor Author

Myself5 commented Jun 30, 2021

Fixed in #373

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.

Vulnerabilities in dependencies
3 participants