-
Notifications
You must be signed in to change notification settings - Fork 3
Code review improvements #175
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
Closed
Changes from all commits
Commits
Show all changes
40 commits
Select commit
Hold shift + click to select a range
b798933
Code review improvements
mernst 2fabad0
Tweak types
mernst 031ed4d
Don't use full name
mernst e445712
Adjust expected error
mernst 7116a33
Document 1-based indexing
mernst bb2891a
Add module documentation
mernst e2f077c
More changes
mernst 5e1baf5
Diagnostics
mernst 4e29093
Merge ../rust_verification into module-doc
mernst b2914e9
Merge ../rust_verification into end-line
mernst fef27cc
Merge ../rust_verification-branch-end-line into review-pre
mernst f4c86ff
Merge ../rust_verification-branch-review-pre into review
mernst 263d23d
Tweak documentation
mernst f10b572
Formatting
mernst 866c147
Fix type annotations
mernst d87c18c
Merge ../rust_verification-branch-end-line into review-pre
mernst 5633d65
Merge ../rust_verification-branch-types into review-pre
mernst 517cdf5
Merge ../rust_verification-branch-review-pre into review
mernst 125687f
Improve Python style
mernst 5e3880c
Merge ../rust_verification-branch-style into review-pre
mernst 51bc5d3
Style
mernst 60e84d6
Merge ../rust_verification-branch-review-pre into review
mernst 2ff15b0
Raise `TypeError` for `isinstance()` failure
mernst 6925cf3
Don't change the type of error raised
mernst 60f3167
Merge ../rust_verification-branch-style into review-pre
mernst 2456f0f
Merge ../rust_verification-branch-value-error-to-type-error into revi…
mernst bdb3fa9
Merge ../rust_verification-branch-review-pre into review
mernst 825e8fb
Fix
mernst 9cf9aa8
Merge branch 'main' into end-line
jyoo980 077808f
Merge branch 'main' into end-line
jyoo980 5c2e704
Merge branch 'main' into end-line
jyoo980 051f493
Merge branch 'main' into end-line
jyoo980 d44b4cc
Merge ../rust_verification into end-line
mernst af089ea
Merge ../rust_verification-branch-end-line into review-pre
mernst 3044b9b
Merge ../rust_verification-branch-review-pre into review
mernst c9cdb02
Fixes
mernst e57fbe0
Merge ../rust_verification-branch-end-line into review-pre
mernst 2356f69
Merge ../rust_verification-branch-review-pre into review
mernst 2aeed9e
Merge ../rust_verification into review-pre
mernst 19c3b08
Merge ../rust_verification-branch-review-pre into review
mernst File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
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
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix Ruff warnings by adding a return type and suppressing
FBT001.As per coding guidelines, all code must pass linting with
make checksyielding 0 errors or warnings. Ruff flagged a missing return type (ANN204) and a boolean positional argument (FBT001). Adding-> Noneand# noqa: FBT001resolves this without breaking existing callers.🛠️ Proposed fix
📝 Committable suggestion
🧰 Tools
🪛 Ruff (0.15.21)
[warning] 39-39: Missing return type annotation for special method
__init__Add return type annotation:
None(ANN204)
[warning] 39-39: Boolean-typed positional argument in function definition
(FBT001)
🤖 Prompt for AI Agents
Sources: Coding guidelines, Linters/SAST tools