-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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 ubuntu-24 #18525
Conversation
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.
Copilot reviewed 5 out of 11 changed files in this pull request and generated no comments.
Files not reviewed (6)
- .devcontainer/swift/Dockerfile: Language not supported
- csharp/ql/integration-tests/all-platforms/autobuild/autobuild.csproj: Language not supported
- csharp/ql/integration-tests/all-platforms/autobuild/global.json: Language not supported
- .github/workflows/swift.yml: Evaluated as low risk
- actions/ql/test/query-tests/Security/CWE-094/.github/workflows/self_needs.yml: Evaluated as low risk
- actions/ql/test/query-tests/Security/CWE-094/.github/workflows/test10.yml: Evaluated as low risk
Tip: Copilot code review supports C#, Go, Java, JavaScript, Markdown, Python, Ruby and TypeScript, with more languages coming soon. Learn more
build-and-test-linux: | ||
if: github.repository_owner == 'github' | ||
runs-on: ubuntu-22.04 | ||
steps: | ||
- uses: actions/checkout@v4 | ||
- uses: ./swift/actions/build-and-test |
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.
swift not supported on ubuntu-24, so dropping this test
@@ -3,7 +3,6 @@ | |||
<PropertyGroup> | |||
<OutputType>Exe</OutputType> | |||
<TargetFramework>net5.0</TargetFramework> |
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.
Maybe target net9.0
instead.
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.
I tested it, and upgrading this will require a coordinated PR in semmle-code
and codeql
. In the interest of time, reducing noise and wrapping up ubuntu migration, I'd like to leave the upgrade to net9
off the scope of this PR , and deal with it separatedly , could we do that?
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.
Yes sure!
623b0b6
to
208551c
Compare
.devcontainer/swift/Dockerfile
Outdated
# [Choice] Debian / Ubuntu version (use Debian 11, Ubuntu 18.04/22.04 on local arm64/Apple Silicon): debian-11, debian-10, ubuntu-22.04, ubuntu-20.04, ubuntu-18.04 | ||
FROM mcr.microsoft.com/vscode/devcontainers/cpp:0-ubuntu-22.04 | ||
# [Choice] Debian / Ubuntu version (use Debian 11, Ubuntu 18.04/22.04 on local arm64/Apple Silicon): debian-11, debian-10, ubuntu-22.04, ubuntu-24.04, ubuntu-20.04, ubuntu-18.04 | ||
FROM mcr.microsoft.com/vscode/devcontainers/cpp:0-ubuntu-24.04 |
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.
how does this change relate to your comment "swift not supported on ubuntu-24, so dropping this test" below?
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.
good point, let me see if we can just remove this
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.
removed all files on .devcontainer/swift, thanks for the heads up!
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.
C# LGTM
No changes in queries.
runs-on
labels to update toubuntu-24
.ImplicitUsings
label removed from target framework (suggested in https://stackoverflow.com/questions/70722475/error-cs8773-feature-global-using-directive-is-not-available-in-c-sharp-9-0) since that label is not available in .Net5 for C# 9 and was failing here https://github.com/github/semmle-code/actions/runs/12827227794