-
Notifications
You must be signed in to change notification settings - Fork 327
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 workflows to actions/upload-artifact@v4
#2583
Conversation
Note: I've left a file that tests the old v3 behavior. It's clearly called legacy, and I saw no particular reason to remove it yet as I'm not removing the code that it validates and someone who has access to GHES could conceivably want to test it. This PR gave me enough insight that I could probably write some code to let that workflow skip doing real work unless it was running under GHES, although the lazy test would be to check the server URL as I did in my initial work. |
👋 thanks again for splitting this out @jsoref! The default value for the feature flag is only used when there is no corresponding feature flag value from the GitHub feature flag API: https://github.com/github/codeql-action/blob/main/src/feature-flags.ts#L276-L293 so the default value change won't actually enable the behavior here. I've begun rolling out the feature flag to Note that the PR checks you are updating can be changed to (The PR checks related to debug artifacts, https://github.com/github/codeql-action/blob/main/.github/workflows/debug-artifacts-failure.yml using v4 and https://github.com/github/codeql-action/blob/main/.github/workflows/debug-artifacts-legacy.yml using v3 are unaffected by the default value/feature flag API value because I have set the |
Sigh. The naming is confusing. Actually, the branch name isn't tied either way, so let's make this PR just the workflow changes. |
69d6d2c
to
1c83cd1
Compare
actions/upload-artifact@v4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the contributions, as always 😄
Merge / deployment checklist