Repository navigation
Rust: Update instructions for rust-analyzer update automation - #22827
Conversation
paldepind
commented
Oct 15, 2026
paldepind
commented
- First commit adds new steps about running QL tests and fixing issues that they expose. I've also moved the "add upgrade/downgrade scripts" earlier. The agentic workflow ends when it opens a PR, so the step is now prior to creating a PR and observing CI.
- Second commit removes some parts of the workflow instructions that duplicate.
In particular, add additional steps about re-generating `.expected` files and fixing problems exposed by doing that.
There was a problem hiding this comment.
🟡 Changes recommended
The validation instructions need corrections to reliably check extractor fixes and all expected test results.
3 open findings
What changed in this PR
Updates rust-analyzer maintenance guidance so automation completes local validation and schema work before opening a PR.
Changes:
- Adds QL expectation regeneration and failure-repair steps.
- Moves schema preparation before PR creation.
- Simplifies workflow instructions and refreshes generated metadata.
| File | Description |
|---|---|
rust/updating-rust-analyzer.md |
Adds test-validation steps and reorders PR preparation. |
.github/workflows/update-rust-analyzer.md |
Consolidates instructions around the maintenance guide. |
.github/workflows/update-rust-analyzer.lock.yml |
Updates the generated workflow body hash. |
🧠 Review effort: Max
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| For each separate problem address the issue, regenerate the `.expected` | ||
| files with the command from step 7, and create a commit. |
There was a problem hiding this comment.
I don't think this needs to be said. Of course the extractor must be re-built if its code is changed.
| For each separate problem address the issue, regenerate the `.expected` | ||
| files with the command from step 7, and create a commit. | ||
|
|
||
| Repeat until no `testFailures` remain and the tests pass. |
There was a problem hiding this comment.
I've tweaked the sentence a bit.
2f94c1d to
89fca11
Compare
andersfugmann
left a comment
andersfugmann
left a comment
There was a problem hiding this comment.
LGTM. One comment to consider if you want to follow up on later.
| 7. Check with CI if everything is in order. | ||
| 7. Update `.expected` files by running: | ||
| ``` | ||
| codeql test run --learn --search-path=rust/extractor-pack:. rust/ql/test |
There was a problem hiding this comment.
Nit: Its my experience that copilot might eagerly do this without actually assessing if the changes are acceptable. Consider in a followup PR to strengthen this by instructing copilot that tests and expected files are precious, and changes should only be accepted after careful scrutiny. Do not blindly accept changes to e.g. locations without first understanding if the changes are good.
There was a problem hiding this comment.
My thinking is that in step 7 we want to blindly accept all test changes. This gives a PR where we can observe exactly what the fallouts are. Then in step 8 things should be fixed and only desired changes should be accepted. Does that make sense?
