FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

change to node20 for all actions by nickfyson · Pull Request #2006 · github/codeql-action · GitHub

/ codeql-action Public

change to node20 for all actions - #2006

Merged
nickfyson merged 16 commits into
mainfrom
nickfyson/node-20
Dec 13, 2023
ConversationCommits (16)ChecksFiles changed
Merged

change to node20 for all actions#2006
nickfyson merged 16 commits into
mainfrom
nickfyson/node-20

Conversation

nickfyson commented Nov 22, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

This PR upgrades all the actions to use node20, and bumps the major version to v3.

It also adds a new PR check to confirm the consistency of node versions across all actions, and ensure that we do not change this version in backport PRs to older releases.

Merge / deployment checklist

  • Confirm this change is backwards compatible with existing workflows.
  • Confirm the readme has been updated if necessary.
  • Confirm the changelog has been updated if necessary.

nickfyson added the Update dependencies Trigger PR workflow to update dependencies label Nov 22, 2023
github-actions Bot removed the Update dependencies Trigger PR workflow to update dependencies label Nov 22, 2023

github-actions Bot commented Nov 22, 2023

Copy link
Copy Markdown
Contributor

Pushed a commit to update the checked-in dependencies. Please mark the PR as ready for review to trigger PR checks.

nickfyson marked this pull request as ready for review November 22, 2023 10:56
nickfyson requested a review from a team as a code owner November 22, 2023 10:56
nickfyson marked this pull request as draft November 22, 2023 10:56
aeisenberg reviewed Nov 22, 2023
View reviewed changes

aeisenberg left a comment •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think the changes you made are good for bumping the node version of the actions. However, once we merge this, we can no longer release v2 of the action until we also update the release scripts to what we had before when we released v1 and v2 (now will be v2 and v3). That's a dicey state to be in.

So, either we can recreate the release scripts and merge it first, or we do them at the same time. I can't remember how we did it last time.

Comment thread CHANGELOG.md Outdated
Comment thread foobar.txt Outdated

aeisenberg commented Nov 22, 2023

Copy link
Copy Markdown
Contributor

Right...it was this PR that added the v2->v1->main release steps #995. It was merged before the node version changes were merged in #1000.

aeisenberg commented Nov 23, 2023

Copy link
Copy Markdown
Contributor

This PR #1482 is the revert PR that removed the v1 release process.

nickfyson added the Update dependencies Trigger PR workflow to update dependencies label Dec 7, 2023
github-actions Bot removed the Update dependencies Trigger PR workflow to update dependencies label Dec 7, 2023

github-actions Bot commented Dec 7, 2023

Copy link
Copy Markdown
Contributor

Pushed a commit to update the checked-in dependencies. Please mark the PR as ready for review to trigger PR checks.

nickfyson marked this pull request as ready for review December 7, 2023 16:24
nickfyson marked this pull request as draft December 7, 2023 17:39
nickfyson marked this pull request as ready for review December 7, 2023 17:39
henrymercer reviewed Dec 7, 2023
View reviewed changes

henrymercer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looks good. Let's wait for the CodeQL 2.15.4 release to complete before merging this to reduce the risk of that release.

One potential safety check we could add is a PR check that fails if the release branch is v2 and the Node version of any Actions is Node 20. This could help make sure (a) we don't forget to switch the v2 branch back to running on node16 the first time we release it, and (b) any new Action we add to main runs on node16 in the v2 branch.

henrymercer previously approved these changes Dec 13, 2023
View reviewed changes
Comment thread .github/workflows/pr-checks.yml Outdated
Comment thread .github/actions/check-sarif/action.yml Outdated
henrymercer previously approved these changes Dec 13, 2023
View reviewed changes
henrymercer reviewed Dec 13, 2023
View reviewed changes

henrymercer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Minor suggestions to the new PR check, looks good overall :)

Comment thread .github/workflows/pr-checks.yml Outdated
Comment thread .github/workflows/pr-checks.yml Outdated
Comment thread .github/workflows/pr-checks.yml Outdated
Comment thread .github/workflows/pr-checks.yml Outdated
Comment thread .github/workflows/pr-checks.yml Outdated
Comment thread .github/workflows/pr-checks.yml Outdated
Apply suggestions from code review
c757f9f 
Co-authored-by: Henry Mercer <henry.mercer@me.com>

nickfyson commented Dec 13, 2023

Copy link
Copy Markdown
Contributor Author

NB – The introduction of a matrix for the Check JS job means that Check JS has been replaced by Check JS (16.11) and Check JS (current).

Once this PR is approved and ready to merge I will remove Check JS as a required check, and once merged I will run the update-required-checks.sh script in order to make the new checks required.

nickfyson merged commit 64e61ba into main Dec 13, 2023
nickfyson deleted the nickfyson/node-20 branch December 13, 2023 14:36
This was referenced Dec 13, 2023
Merged
Merged
TWiStErRob mentioned this pull request Dec 14, 2023
Open
This was referenced Dec 18, 2023
Closed
Closed
reesscot mentioned this pull request Mar 18, 2024
Merged
5 tasks
mario-campos added a commit that referenced this pull request Sep 29, 2025
Implement simultaneous PR checks for Node.js v20, v24.
1ede03a 
Copied from #2006.
mario-campos added a commit that referenced this pull request Sep 30, 2025
Implement simultaneous PR checks for Node.js v20, v24.
a4ab3ec 
Copied from #2006.
mario-campos added a commit that referenced this pull request Sep 30, 2025
Implement simultaneous PR checks for Node.js v20, v24.
f730099 
Copied from #2006.
mario-campos added a commit that referenced this pull request Sep 30, 2025
Implement simultaneous PR checks for Node.js v20, v24.
d4bbcb7 
Copied from #2006.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

aeisenberg aeisenberg left review comments
henrymercer henrymercer approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL