Improve Rust analysis PR check #3023
Conversation
Also run the `rust` checks on "milestone" CLI releases, to ensure we remain backward compatible with those versions. This was prompted by #2960 (review) Running this on current `main` and then on that PR should improve our confidence we remain backward compatible. It also turns out a probable `ruamel.yaml` update was changing a lot of generated workflows, so I've: * fixed the `ruamel.yaml` version to the latest in `sync.sh` * added `yaml.width = 120` in `sync.py` to minimize (but not entirely remove) the number of changes * checked in the workflows whose formatting was changed by the new `ruamel.yaml` version
There was a problem hiding this comment.
Pull Request Overview
This PR improves the Rust analysis PR check by expanding test coverage to include milestone CLI releases and fixes workflow generation formatting issues. The changes ensure backward compatibility testing with historical CodeQL versions while stabilizing the workflow generation process.
Key changes:
- Adds testing for milestone Rust support versions (v2.19.3 and v2.22.1) alongside existing versions
- Fixes ruamel.yaml version and formatting configuration to prevent unwanted workflow changes
- Updates generated workflows with improved formatting consistency
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| pr-checks/sync.sh | Pins ruamel.yaml to version 0.18.14 for consistent behavior |
| pr-checks/sync.py | Adds yaml.width = 120 configuration for better line formatting |
| pr-checks/checks/rust.yml | Expands test matrix to include milestone Rust support versions and removes deprecated environment variable |
| .github/workflows/__start-proxy.yml | Reformats registry_secrets and conditional statements for better readability |
| .github/workflows/__rust.yml | Adds new test matrix entries for milestone versions and removes deprecated environment variable |
| .github/workflows/__resolve-environment-action.yml | Simplifies conditional statement formatting |
| .github/workflows/__packaging-inputs-js.yml | Improves line wrapping for long packs parameter |
| .github/workflows/__multi-language-autodetect.yml | Simplifies conditional expression formatting |
Comments suppressed due to low confidence (1)
pr-checks/checks/rust.yml:5
- The CodeQL versions stable-v2.19.3 and stable-v2.22.1 may not exist or be valid. Please verify these are actual released versions of CodeQL CLI that support Rust analysis.
- stable-v2.19.3 # experimental rust support introduced, requires action to set `CODEQL_ENABLE_EXPERIMENTAL_FEATURES`
- stable-v2.22.1 # first public preview version
pr-checks/checks/rust.yml
Outdated
| - stable-v2.19.3 # experimental rust support introduced, requires action to set `CODEQL_ENABLE_EXPERIMENTAL_FEATURES` | ||
| - stable-v2.22.1 # first public preview version |
There was a problem hiding this comment.
Assuming this is what primarily motived the the yaml.width change, I think you could just move the comments above the relevant list entries to avoid the problem?
There was a problem hiding this comment.
no, the yaml.width changes the output workflows, does not take the inputs.
What I noticed is that there were a lot of new line-breaks all over the generated workflows. I thought that if I entered the "right" width I would get no changes, but couldn't find any such value. In the end I settled for a reasonable value that limited changes to only a couple of workflows.
There was a problem hiding this comment.
I think it would still be nicer to move the comments above the list elements. That's what we do elsewhere and it means you don't need such a wide code view / scrolling.
Do you have an example of what you mean by "a lot of new line-breaks all over the generated workflows"? Looking over a random sample didn't show anything odd.
There was a problem hiding this comment.
Or is that a result of updating ruamel.yaml? I had assumed that the update was to have yaml.width, but maybe I made the wrong assumption.
There was a problem hiding this comment.
yeah, updating ruamel.yaml without specifying the width was breaking up lines more often than the previous version. In the end though I opted for using the version that was specified (elsewhere than sync.sh, which is what I was using), but also fix up that so that now the version is only written once. I will move the comments now.
There was a problem hiding this comment.
Possibly a result of https://sourceforge.net/p/ruamel-yaml/tickets/529/. That's the only width-related change I saw in the changelog after 0.17.31
Also run the
rustchecks on "milestone" CLI releases, to ensure we remain backward compatible with those versions. This was prompted by #2960 (review)Running this on current
mainand then on that PR should improve our confidence we remain backward compatible.It also turns out a probable
ruamel.yamlupdate was changing a lot of generated workflows locally. It turns out the version was fixed in a couple of places but not in thesync.shscript I used. I've opted for writing the version exclusively in there and use it all over, which did mean updating the headers in all generated workflows to reflect the new instructions not specifyingruamel.yaml's version.Merge / deployment checklist