Repository navigation
Conversation
kuhe
marked this pull request as ready for review
November 10, 2026 16:37
kuhe
force-pushed
the
kuhe/chore/more-static-analysis
branch
3 times, most recently
from
November 10, 2026 20:05
85e5ab9 to
7c6dfa3
Compare
kuhe
force-pushed
the
kuhe/chore/more-static-analysis
branch
from
November 10, 2026 20:10
7c6dfa3 to
f73d639
Compare
ubaskota
reviewed
Nov 10, 2026
| }, | ||
| ) | ||
| lines = checked.stdout.splitlines() | ||
| return [line for line in lines if " - error: " in line] |
There was a problem hiding this comment.
If pyright ever fails with exit code 2–4, the failure goes unnoticed with the current check. Should we also verify the exit code?
| roots = [str(p.src) for p in packages()] | ||
|
|
||
| def one(name: str) -> tuple[str, dict[str, Any]]: | ||
| proc = run([python, str(PROBE), name, *roots], timeout=300) |
There was a problem hiding this comment.
What happens if a probe runs for more than 300 seconds? Would that crash the whole check?
Contributor
Author
There was a problem hiding this comment.
I think individual checks should be considered in error (either code is wrong or the check is wrong) if they run for that long.
For legitimately slow things like typechecking a huge client, exceptions should be made.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As with smithy-lang/smithy-python#898, this adds a static (loosely defined) analysis verification suite.
FAQ: this is a lot of duplication of code. Won't it drift?
Yes, but it's ok. It's not runtime code, and it allows this code to differ intentionally from smithy-python's verification suite when needed. It avoids the creation of a package containing this code to be publicly maintained and accept slight variations between the two repositories just for DRY.
It allows the code to be edited in the same PR that changes any of the assumptions.
Note: the PR does not fix the violations revealed by the suite. That's for later.
Approving the PR doesn't even necessarily mean you accept all the verifications up front. Their enforcement PRs will be reviewed separately and also present opportunities to tweak the rules.