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.

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

Fix stdout being plugged into codemod-ed file #309

Merged
merged 1 commit into from
Jun 10, 2020

Conversation

josieesh
Copy link
Contributor

@josieesh josieesh commented Jun 9, 2020

Summary

  • When running a codemod on a file, the output success message of black ended up embedded in the file itself. See image below:
    image

  • Tried to set the -q flag here to quiet the output but that ended up completely erasing the test file

  • Removing the line of code in libcst/codemod/_cli.py did the trick. The file still gets reformatted and the black output message still ends up in the terminal. See image below:
    image

Test Plan

  • Tested a test file z.py with various existing codemod commands
  • Tested with --no-format flag to ensure that when it's on, black is not called at all

@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 Jun 9, 2020
@josieesh josieesh marked this pull request as ready for review June 9, 2020 21:09
Copy link
Contributor

@jimmylai jimmylai left a comment

Choose a reason for hiding this comment

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

Nice catch!
Set stderr=subprocess.STDOUT makes stderr output been captured as updated code.

We didn't run into this issue internally because we adds -q parameter to silence stderr when calling Black.

@josieesh josieesh changed the title Fix stderr being plugged into codemod-ed file Fix stdout being plugged into codemod-ed file Jun 9, 2020
@codecov-commenter
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@ed1a2ad). Click here to learn what that means.
The diff coverage is n/a.

@josieesh josieesh merged commit 228589f into Instagram:master Jun 10, 2020
@josieesh josieesh deleted the fix_codegen branch June 10, 2020 16:46
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