Skip to content

Conversation

@TrojanerHD
Copy link
Collaborator

@TrojanerHD TrojanerHD commented Aug 29, 2023

  • Upgrade gradle wrapper
  • Upgrade dependencies and plugins
  • Force trailing commas
  • Set compatibility version of java instead of pinning the JDK version using toolchain

@rubengees
Copy link
Member

Before I review this in detail: I think for this project it would be good to remain backwards compatible with older Java versions. I would like to continue supporting Java 8 if it does not become too much of a pain. That way consumers with older projects can continue using this.

@TrojanerHD TrojanerHD requested a review from rubengees September 6, 2023 12:45
Copy link
Member

@rubengees rubengees left a comment

Choose a reason for hiding this comment

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

Good work!

Needs a rebase and we need to check the frontend dependencies, check my comments. Since a bit of time has passed we can also already update some dependencies again I think. Gradle 8.4 has released e.g.


bootstrapVersion = '4.6.0'
jqueryVersion = '3.6.0'
bootstrapVersion = '5.3.1'
Copy link
Member

Choose a reason for hiding this comment

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

If we want to upgrade this we also need to update the frontend since this version is not backwards compatible.

Comment on lines +28 to +32
jqueryVersion = '3.7.0'
popperVersion = '1.16.1'
fontAwesomeVersion = '5.15.4'
diff2htmlVersion = '3.1.7'
markedVersion = '2.0.6'
fontAwesomeVersion = '6.4.2'
diff2htmlVersion = '3.4.29'
markedVersion = '7.0.3'
Copy link
Member

Choose a reason for hiding this comment

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

Also manually test the other frontend dependencies.

Comment on lines +106 to +107
"\"${result.description}\""
.replace("\n", HTML_LINE_ENDING)
Copy link
Member

Choose a reason for hiding this comment

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

We can have this in a single line now I think.

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