-
Couldn't load subscription status.
- Fork 13.9k
Migrate unused-revision-names check from tidy to compiletest #148078
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
Conversation
|
Some changes occurred in src/tools/compiletest cc @jieyouxu |
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
4a1ea26 to
e6820bf
Compare
|
Some changes occurred in src/tools/cargo cc @ehuss |
|
|
Somehow this PR has ended up including a bunch of submodule changes, making it unreviewable in its current state. |
|
Given that the tidy check currently works fine, and considering the current state of compiletest directive handling, I don't think we should make this change at this time. Adding more complexity to directive-handling code that is in dire need of major overhauls will only serve to make those overhauls even harder, without any significant benefit. |
|
I'm inclined to agree, getting directive handling code out of its current situation is a much higher priority. I don't want to introduce complications for cleanup efforts on that front in the short/medium term. Thanks for the PR in any case. |
From what i understand, we need to migrate the check such that it executes before running the actual tests(revisions) as the runtest::run() assumes that theres no revision/unused-revision related issue.
Implementation/Logic:
(I initially thought to implement it in TestProps then execute it in runtest::run() but that would be too late).
Tests:
I've tried to keep its functioning as close to what it was in tidy as possible.
I'm not super sure if this is most optimal solution,
so any feedback on approach or potential edge cases would be very helpful.
Issue: #144671