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

feat: add --check CLI option #64

Merged
merged 5 commits into from May 20, 2019
Merged

feat: add --check CLI option #64

merged 5 commits into from May 20, 2019

Conversation

smably
Copy link
Contributor

@smably smably commented May 15, 2019

My use case: I want to run prettier --check on CI, but in order to speed up my CI tasks, I'd rather not check every file in my codebase on every commit. Ideally I'd like to be able to run prettier --check, but just on the files changed in the current branch.

This is really similar to what pretty-quick --bail does, but I don't actually want to format the files, I just want pretty-quick to tell me if they need formatting.

This PR adds a --check CLI flag that runs prettier.check instead of prettier.format and exits with a non-zero status code if any files fail the check.

I also renamed onWriteFile to onProcessFile and formatFiles.js to processFiles.js to reflect that they now handle either formatting or checking operations. This change is in a separate commit to keep the diff for the main code change a bit cleaner.

Copy link
Member

@azz azz left a comment

Choose a reason for hiding this comment

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

Thanks for implementing this! One minor change requested.

src/processFiles.js Outdated Show resolved Hide resolved
src/processFiles.js Outdated Show resolved Hide resolved
@smably
Copy link
Contributor Author

smably commented May 16, 2019

I tested this a bit more and realized my initial implementation was buggy. 😳

Fixed this up a bunch, renamed a few things, also made it so only the failed filenames are printed (same as how filenames are only printed when files are written).

Copy link
Member

@azz azz left a comment

Choose a reason for hiding this comment

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

LGTM, just one rename requested.

src/index.js Outdated Show resolved Hide resolved
@azz azz merged commit a6057ce into prettier:master May 20, 2019
@azz
Copy link
Member

azz commented May 20, 2019

Thanks!

@smably smably deleted the check branch May 20, 2019 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants