Dedupe Variable construction in pyrefly coverage#3772
Conversation
This comment has been minimized.
This comment has been minimized.
|
Thanks @jorenham, this is a satisfying cleanup. Folding the five Variable push sites into one let (annotation, slots) = match … and pulling out classify_annotation reads much better, and is_schema_class returning a bool is a nice simplification over threading the ClassBinding back out. LGTM and importing now |
|
@NathanTempest has imported this pull request. If you are a Meta employee, you can view this in D108307259. |
jorenham
force-pushed
the
coverage/dedupe-Variable-construction
branch
from
3dc74c7 to
091ed8b
Compare
|
Rebased onto main to resolve merge conflicts |
|
The CI failures are unrelated |
This comment has been minimized.
This comment has been minimized.
Summary: ref: #3772 (comment) Pull Request resolved: #3786 Test Plan: The classic "wait and see if CI works now" approach. Reviewed By: maggiemoss Differential Revision: D108446033 Pulled By: NathanTempest fbshipit-source-id: 30c4a47f15269d2b72be4578175189866b334161
yangdanny97
left a comment
yangdanny97
left a comment
There was a problem hiding this comment.
Review automatically exported from Phabricator review in Meta.
jorenham
force-pushed
the
coverage/dedupe-Variable-construction
branch
from
091ed8b to
cb8d2da
Compare
|
rebased to fix CI |
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
|
@NathanTempest merged this pull request in 401aa37. |
3 participants
Summary
This is another
pyrefly coverage-related refactor that dedupes the variable creation code inparse_variables, which was repeated 5 times. and simplifies some other related code.This shouldn't result in any merge conflicts with the other (currently open)
coveragerefactor PRs.Test Plan
Tests still pass (no behavior changes).