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

Improve formatting #1732

Merged
merged 1 commit into from Feb 11, 2019

Conversation

2 participants
@kt3k
Copy link
Contributor

kt3k commented Feb 10, 2019

This PR fixes several errors about the formatting script and improves the logging when something errors.

  • --allow-read was necessary for std/prettier/main.ts
  • yapf has been failing since format.ts was created. (It doesn't find python_packages path in third_party)
  • Error code of each formatter was ignored. I changed format.ts to exit with error when one of the formatters exits with non-zero status code.
  • I added error logging when one of the formatters failed. This would be useful for debugging the flaky behavior of format.ts in CI. cf #1718

@kt3k kt3k force-pushed the kt3k:feature/improve-format branch from 67cf17a to 5d6fa71 Feb 10, 2019

@ry

ry approved these changes Feb 11, 2019

Copy link
Collaborator

ry left a comment

LGTM

@@ -19,7 +19,8 @@ Before submitting, please make sure the following is done:

1. There are tests that cover the changes.
2. Ensure `./tools/test.py` passes.
3. Format your code with `deno ./tools/format.ts --allow-read --allow-run`.
3. Format your code with `PYTHONPATH=third_party/python_packages deno ./tools/format.ts --allow-read --allow-run`.
<!-- TODO: set PYTHONPATH in format.ts when run API has env option -->

This comment has been minimized.

@ry

ry Feb 11, 2019

Collaborator

Ok better to have this TODO than nothing - but this is a painful command to type. We need fix this soon.

@ry ry merged commit d266553 into denoland:master Feb 11, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
license/cla Contributor License Agreement is signed.
Details

@kevinkassimo kevinkassimo referenced this pull request Feb 11, 2019

Merged

Fix REPL formatting #1744

@kt3k kt3k deleted the kt3k:feature/improve-format branch Feb 11, 2019

@kt3k kt3k referenced this pull request Feb 15, 2019

Merged

Add env option to run api #1773

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment