Skip to content

Comments

Upgrade to µsort 1.0.0rc1, and apply formatting changes#565

Merged
amyreese merged 3 commits intoInstagram:mainfrom
amyreese:usort-1.0
Dec 21, 2021
Merged

Upgrade to µsort 1.0.0rc1, and apply formatting changes#565
amyreese merged 3 commits intoInstagram:mainfrom
amyreese:usort-1.0

Conversation

@amyreese
Copy link
Contributor

Summary

Upgraded µsort and µfmt in requirements. Ran ufmt format *.

Test Plan

Manual diff inspection and CI

@amyreese amyreese requested a review from zsol December 16, 2021 21:50
@amyreese amyreese self-assigned this Dec 16, 2021
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Dec 16, 2021
@amyreese amyreese requested a review from thatch December 16, 2021 21:51
Comment on lines 94 to 103
def __repr__(self) -> str:
...


def detect_encoding(readline: Callable[[], bytes]) -> Tuple[str, Sequence[bytes]]:
...


def tokenize(readline: Callable[[], bytes]) -> Generator[TokenInfo, None, None]:
...
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this could/should be reported as a bug on µfmt for not detecting/telling black that it's formatting a type stub.

@amyreese
Copy link
Contributor Author

@zsol I don't see any excludes configured in pyproject.toml, but you mentioned them on facebook/usort#62; are those needed to get a clean CI pass?

@zsol
Copy link
Contributor

zsol commented Dec 17, 2021

Ah, I had to add them in the rust branch over here: https://github.com/zsol/LibCST/blob/parser/pyproject.toml#L5-L6

@zsol
Copy link
Contributor

zsol commented Dec 17, 2021

There are some relevant changes you'll need to do from zsol@6925202 to get the failing tests to pass. Check out test_codegen_clean.py in that commit. Unfortunately ufmt will not run on files that end in .deleteme so the tests will always compare unformatted generated code with the formatted checked-in version. That took me a while to track down.

@codecov-commenter
Copy link

Codecov Report

Merging #565 (fadeb64) into main (c02de9b) will not change coverage.
The diff coverage is 96.92%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #565   +/-   ##
=======================================
  Coverage   94.62%   94.62%           
=======================================
  Files         240      240           
  Lines       23389    23389           
=======================================
  Hits        22132    22132           
  Misses       1257     1257           
Impacted Files Coverage Δ
libcst/_batched_visitor.py 94.54% <ø> (ø)
libcst/_metadata_dependent.py 92.30% <ø> (ø)
libcst/_parser/base_parser.py 86.58% <ø> (ø)
libcst/_parser/entrypoints.py 79.59% <0.00%> (ø)
libcst/_parser/py_whitespace_parser.py 95.61% <ø> (ø)
libcst/codemod/__init__.py 100.00% <ø> (ø)
libcst/codemod/tests/test_runner.py 98.00% <ø> (ø)
...emod/visitors/tests/test_apply_type_annotations.py 100.00% <ø> (ø)
libcst/matchers/_return_types.py 100.00% <ø> (ø)
libcst/matchers/_visitors.py 97.03% <ø> (ø)
... and 63 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c02de9b...fadeb64. Read the comment docs.

@zsol
Copy link
Contributor

zsol commented Dec 21, 2021

I rebased after merging the commit mentioned above, re-applied formatting and codegen changes to the PR.

Copy link
Contributor

@zsol zsol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good to me, feel free to merge if CI is green

@amyreese amyreese merged commit 10c3aa0 into Instagram:main Dec 21, 2021
@amyreese amyreese deleted the usort-1.0 branch December 21, 2021 22:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants