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 the GitHub Actions workflows to use the current Node.js LTS version #17201

Merged
merged 1 commit into from
Nov 2, 2023

Conversation

Snuffleupagus
Copy link
Collaborator

The active LTS version is now based on Node.js version 20, hence let's update the relevant workflows to use that one instead; see https://en.wikipedia.org/wiki/Node.js#Releases

Given that we still support Node.js version 18, i.e. the maintenance LTS version, in the PDF.js library we'll keep testing both versions in GitHub Actions to prevent regressions.

…rsion

The active LTS version is now based on Node.js version 20, hence let's update the relevant workflows to use that one instead; see https://en.wikipedia.org/wiki/Node.js#Releases

Given that we still support Node.js version 18, i.e. the maintenance LTS version, in the PDF.js library we'll keep testing both versions in GitHub Actions to prevent regressions.
@Snuffleupagus
Copy link
Collaborator Author

I don't understand why the "Test"-task no longer reports success, since both jobs passed, but I'm probably missing something regarding the configuration...

@timvandermeij
Copy link
Contributor

timvandermeij commented Oct 29, 2023

The configuration looks good, but I think GitHub got confused somehow, perhaps because of force-pushes while the actions were still running. I have retriggered them to verify this.

@timvandermeij
Copy link
Contributor

timvandermeij commented Oct 29, 2023

Reading threads like https://github.com/orgs/community/discussions/26698, https://github.com/orgs/community/discussions/30724 and https://stackoverflow.com/a/71983537 it looks like it's because the matrix approach changes the names of the test steps, and the fact that the currently named task is marked as required while the matrix-named tasks are not indicates that we need to change the branch protection rule configuration for this repository. Sadly only administrators can do this, so maybe @calixteman can chime in for this; could you perhaps check the branch protection rules for this repository and update it to the new names? (Hopefully we can do this with some kind of regex like "Test.*" so that we don't have to change this next time a new test name gets introduced if the matrix is changed.)

Copy link
Contributor

@timvandermeij timvandermeij left a comment

Choose a reason for hiding this comment

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

The change itself is correct, so I'm approving this, but we can only merge it once the branch protection rules have been updated.

@Snuffleupagus Snuffleupagus merged commit 20adb2c into mozilla:master Nov 2, 2023
7 checks passed
@Snuffleupagus Snuffleupagus deleted the node-ci-lts branch November 2, 2023 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants