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: Fail on fix (#52) #53

Closed
wants to merge 1 commit into from
Closed

Conversation

josejulio
Copy link

The purpose of this is to have something that aborts commits in case any file is fixed.

I added a flag to maintain backwards compatibility, so only users who pass the --fail-on-fix will have this behavior.

@aleclarson
Copy link

Good work! I vote for the name being --bail like Jest does it.

Please remove the object being passed to mri. We don't use that for any of the other flags.

So this can implemented as a one-liner:

 onWriteFile: file => {
   console.log(`✍️  Fixing up ${chalk.bold(file)}.`);
+  if (args.bail) success = false;
 }

@josejulio
Copy link
Author

Thanks for the name suggestion, i was looking for something short to avoid the mri flags :)

README.md Outdated
@@ -92,6 +92,10 @@ When not in `staged` pre-commit mode, use this flag to compare changes with the

Outputs the name of each file right before it is proccessed. This can be useful if Prettier throws an error and you can't identify which file is causing the problem.

## `--fail-on-fix`

Makes pretty-quick to exit with a non-zero exit code if any file is fixed. The intent is to abort the git commit.

Choose a reason for hiding this comment

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

Change to this please:

## `--bail`

Prevent `git commit` if any files are fixed.

@josejulio
Copy link
Author

@aleclarson done

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.

We need a different error message when bailing, the current one is

Partially staged files were fixed up

Which is not correct in this case.

Could you also add a test?

@josejulio
Copy link
Author

We need a different error message when bailing, the current one is

Partially staged files were fixed up

Which is not correct in this case.

Could you also add a test?

Sure, will attempt to add it next week, thanks!

@jantimon
Copy link
Collaborator

Added unit tests to #60

@josejulio
Copy link
Author

Thanks @jantimon !

@josejulio josejulio closed this Jan 17, 2019
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

4 participants