-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[red-knot] Move type-property tests to Markdown #15397
Comments
Why only one? Wouldn't it be easier for both the contributor and the reviewers to have one instead of multiple PRs? These are closely related, after all. |
They are related tasks of course, but they can be cleanly separated. The Rust code in these tests changes from time to time (the latest refactoring that changed these was yesterday), and new test cases are added. So if we merge all of these into one giant contribution, it increases the risk for annoying merge conflicts that could be avoided otherwise. As a maintainer, I also prefer multiple smaller contributions. That said, obviously I won't complain if someone wants to tackle more or all of these at once. |
I strongly agree that small, incremental PRs are much easier to review, and therefore much more helpful |
## Summary See title. Part of #15397 ## Test Plan Ran new Markdown test. --------- Co-authored-by: Alex Waygood <[email protected]>
## Summary Part of #15397. ## Test Plan Markdown tests. --------- Co-authored-by: David Peter <[email protected]>
…` unit tests to Markdown tests (#15533) ## Summary Part of #15397. ## Test Plan Markdown tests. --------- Co-authored-by: Alex Waygood <[email protected]>
The various type-property tests here …
ruff/crates/red_knot_python_semantic/src/types.rs
Lines 4155 to 4673 in b861551
… can now be moved to Markdown-based tests which are more concise, easier to read, can be run without recompilation, and can contain additional prose for documentation.
An example on how to do this can be seen in this PR which moved the
is_assignable_to
tests (ignore the changes inproperty_tests.rs
).If you would like to work on this, let us know and pick one of these:
tuple_containing_never_simplifies_to_never
is_assignable_to
is_subtype_of
is_equivalent_to
is_disjoint_from
is_singleton
is_single_valued
is_fully_static
The text was updated successfully, but these errors were encountered: