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

Update dependencies of the Java CI workflow #33

Merged
merged 3 commits into from
Aug 8, 2024

Conversation

aureliony
Copy link

Closes #26.

Copy link

@drustanyjt drustanyjt left a comment

Choose a reason for hiding this comment

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

Tested the CI workflows and can verify that warnings no longer appear when it is run:
image
LGTM

@damithc
Copy link

damithc commented Jul 26, 2024

@aureliony either send separate PRs for each independent change, or do all in this PR but have a separate commit for each independent change.

Copy link

@brein62 brein62 left a comment

Choose a reason for hiding this comment

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

LGTM! Checked your branch's CI workflow and confirmed that no warnings appear. Thank you so much for running and testing the changes on your end.

update-workflow-screenshot

with:
distribution: 'zulu'
Copy link

Choose a reason for hiding this comment

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

For your information as to why Zulu has to be specified here for setup-java@v4:

When upgrading actions/setup-java@v1 to actions/setup-java@v4, from v2 onwards, the distribution parameter here is mandatory. Since the original CI workflow version (v1) did not specify a distribution parameter, it used the zulu distribution by default (https://github.com/actions/setup-java/blob/main/docs/switching-to-v2.md).

Copy link

Choose a reason for hiding this comment

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

Thanks for this info, @brein62
@aureliony Perhaps mention this in the relevant commit message? Also, your commit message body is slightly wider than permitted by our Git convention.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the additional info. Commit bodies have been updated and wrapped at 72 chars.

The current version (v1) of Gradle Wrapper Validation Action
is outdated and may stop working in the future.

Let's update the workflow dependency, and
use the format as recommended in
https://github.com/gradle/wrapper-validation-action/blob/main/README.md.
The current version (v1) of the setup-java action is outdated,
and may stop working in the future. As newer versions of the action
require the 'distribution' field, 'zulu' is specified, which is
the default used in v1.

Let's update the workflow dependency, and use the format as recommended
in https://github.com/actions/setup-java/blob/main/README.md.
The current version (v3) of the codecov action had previously stopped
working (nus-cs2103-AY2324S2/forum#729), and
may stop working again in the future.

Let's update the codecov action to the latest version (v4) to minimize
the chance of it breaking, and use the format as recommended in
https://github.com/codecov/codecov-action/blob/main/README.md.
@damithc damithc merged commit 93e00db into nus-oss:master Aug 8, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update CI script to match newer versions of the tools used
4 participants