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

Upgrade deps #605

Merged
merged 5 commits into from
Aug 8, 2022
Merged

Upgrade deps #605

merged 5 commits into from
Aug 8, 2022

Conversation

HEdingfield
Copy link
Contributor

@HEdingfield HEdingfield commented Aug 7, 2022

Upgrades Gradle to 7.5.1, spotbugs-gradle-plugin to 5.0.9, spotbugs to 4.7.1, checkstyle to 10.3.2, jlink to 2.25.0, jackson to 2.13.3, jupiter to 5.9.0, and javafx to 18. Also fixes unchecked Java compiler warning and issue where copyright was being inserted in non-source files.

NOTE:

  • I decided not to update to JDK 18 (yet) because JDK 17 is LTS
  • I wanted to update org.openjfx.javafxplugin to 0.0.13, but was blocked by this issue
  • Suppressions were initially ignored after updating Gradle to 7.5.1, so I had to manually apply the fix mentioned here in google_checks.xml

@HEdingfield HEdingfield requested review from tarheel and moldover August 7, 2022 17:22
@moldover
Copy link
Contributor

moldover commented Aug 7, 2022

Love it. Would you mind throwing one more in? Update javafx to version 18 in build.gradle:

// ### JavaFX plugin settings
javafx {
version = "18"
modules = ["javafx.base", "javafx.controls", "javafx.fxml", "javafx.graphics"]
}

This is actually a =problem for me because the GUI crashes on M1 :/ Fixed in version 18.
Discussion:

…iscellaneous clean-up in `DominionCvrReader`.
@HEdingfield
Copy link
Contributor Author

Updated!

@HEdingfield HEdingfield requested a review from tamird August 7, 2022 19:24
// Keep the below file updated along with subsequent versions of Checkstyle (make sure to choose
// the tag matching the toolVersion above):
// https://github.com/checkstyle/checkstyle/blob/checkstyle-10.2/src/main/resources/google_checks.xml
// https://github.com/checkstyle/checkstyle/blob/checkstyle-10.3.2/src/main/resources/google_checks.xml
// NOTE: It may also be necessary to change "${org.checkstyle.google.suppressionfilter.config}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't understand this note.

EDIT: ah, I see that this changed in the file itself. Can you clarify this comment to:

  • cite something
  • indicate that the edit takes place in that file?

Also, was that file updated by just downloading a new one? It might be good to write down a curl command here; you could even write curl ... | sed s|org.checkstyle...|\${config_log}/...| to automate this away entirely.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added more context per your request. Yeah, I just copy and paste it from GH usually, and I like the idea of automating it with a script like that except the address would need to be changed each time to the match most recent version posted to the Maven repo, and sed is a no-go in my Windows environment. :/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ack, sorry, missed this PR with this change. Will include it in my next.

@@ -1,5 +1,16 @@
#
# RCTab
Copy link
Collaborator

Choose a reason for hiding this comment

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

isn't there a gradle command that generates these files? if yes, it may not make sense to put these license headers there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, this definitely shouldn't be appearing in here. I'll fix the auto-copyright rules and clean these up.

@HEdingfield HEdingfield merged commit b08177a into develop Aug 8, 2022
@HEdingfield HEdingfield deleted the upgrade-deps branch August 8, 2022 08:05
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.

3 participants