Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix black configuration #769

Merged
merged 8 commits into from
Sep 14, 2022
Merged

Fix black configuration #769

merged 8 commits into from
Sep 14, 2022

Conversation

MapleCCC
Copy link
Contributor

@MapleCCC MapleCCC commented Sep 6, 2022

No description provided.

@MapleCCC
Raise black's output file's target version to 3.7, which is the lowes…
6752a20 
…t supported Python version that libcst can be run on
@MapleCCC
Add to, instead of override, the exclusion rules of black
38656c6
@MapleCCC
Fix the bug that files in stubs/libcst_native/ are inadvertently ig…
3935e30 
…nored by black

This is due to black's file exclusion mechanism is a file-system-unaware pure-string-based pattern match. We need to prepend "^/" to specify that we are referring to the root-level "native/" folder. Yeah, I know this looks strange, but blame black for it :) . See https://black.readthedocs.io/en/stable/usage_and_configuration/the_basics.html#configuration-format for further reference.
@MapleCCC
It's conventional to use single-quote literal string to represent reg…
be878a2 
…ular expression in TOML format, because in this way it doesn't perform any escaping
@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 Sep 6, 2022
@MapleCCC
Copy link
Contributor Author

MapleCCC commented Sep 6, 2022 •
edited
Loading

The CI error is due to a unit test test_codemod_formatter_error_input trying to test that, when libcst.codemod is run on a file containing async/await reserved keywords, it's expected to always fail trying to apply the black formatter on the file.

The formatter error turns out to be a total unfortunate incident, relying on black incidentally picking up the target version configuration (i.e., "py36") in our own pyproject.toml simply because it happens to be run in our root folder.

This is so scary in many ways :-) . I think we can all agree on that this unit test is problematic, and needs some proper fix ?

@MapleCCC MapleCCC mentioned this pull request Sep 6, 2022
Merged
@MapleCCC
Copy link
Contributor Author

MapleCCC commented Sep 6, 2022

For the record, another PR #771 has been opened to address the CI errors we see here.

@MapleCCC
Copy link
Contributor Author

MapleCCC commented Sep 6, 2022

Now I am going to merge #771 into us here, so that we can have an all-green CI pass.

@MapleCCC
Copy link
Contributor Author

MapleCCC commented Sep 6, 2022

I believe the CI error is GitHub's fault, not ours :-)

@zsol zsol merged commit c75dbd4 into Instagram:main Sep 14, 2022
@zsol
Copy link
Member

zsol commented Sep 14, 2022

Thanks!

@MapleCCC MapleCCC deleted the fix-black-configuration branch September 14, 2022 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@MapleCCC @zsol @facebook-github-bot