Skip to content

Improve Rust analysis PR check #3023

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Aug 12, 2025
+70 −69
Merged

Improve Rust analysis PR check #3023

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
28f2516
Improve Rust analysis PR check
redsun82 Aug 11, 2025
2d7401b
Revert `ruamel.yaml` back to 0.17.31
redsun82 Aug 11, 2025
286b9e9
Specify the `ruamel.yaml` version in one place only (`sync.sh`)
redsun82 Aug 11, 2025
bf1dd69
Move comments up in `rust.yml`
redsun82 Aug 11, 2025
9f966bb
Merge branch 'main' into redsun82/rust-test
redsun82 Aug 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions .github/workflows/__multi-language-autodetect.yml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion .github/workflows/__packaging-inputs-js.yml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions .github/workflows/__resolve-environment-action.yml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 4 additions & 2 deletions .github/workflows/__rust.yml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 3 additions & 4 deletions .github/workflows/__start-proxy.yml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 6 additions & 3 deletions pr-checks/checks/rust.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
name: "Rust analysis"
description: "Tests creation of a Rust database"
versions: ["linked", "default", "nightly-latest"]
versions:
- stable-v2.19.3 # experimental rust support introduced, requires action to set `CODEQL_ENABLE_EXPERIMENTAL_FEATURES`
- stable-v2.22.1 # first public preview version
Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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

- linked
- default
- nightly-latest
operatingSystems: ["ubuntu"]
steps:
- uses: ./../action/init
with:
languages: rust
tools: ${{ steps.prepare-test.outputs.tools-url }}
env:
CODEQL_ACTION_RUST_ANALYSIS: true
- uses: ./../action/analyze
id: analysis
with:
Expand Down
1 change: 1 addition & 0 deletions pr-checks/sync.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ def writeHeader(checkStream):
yaml = ruamel.yaml.YAML()
yaml.Representer = NonAliasingRTRepresenter
yaml.indent(mapping=2, sequence=4, offset=2)
yaml.width = 120

this_dir = pathlib.Path(__file__).resolve().parent

Expand Down
2 changes: 1 addition & 1 deletion pr-checks/sync.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@ set -e
cd "$(dirname "$0")"
python3 -m venv env
source env/bin/activate
pip3 install ruamel.yaml
pip3 install ruamel.yaml==0.18.14
python3 sync.py

Loading