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

Add prospector as a pre-commit hook #3328

Merged
merged 1 commit into from Dec 1, 2017

Conversation

@humitos
Copy link
Member

@humitos humitos commented Nov 28, 2017

To continue with #3264 , I added prospector to the pre-commit hooks

@ericholscher
Copy link
Member

@ericholscher ericholscher commented Nov 28, 2017

Will this delay the actual committing by the prospector run? Could see that being mildly annoying in some cases.

@humitos
Copy link
Member Author

@humitos humitos commented Nov 28, 2017

Kind of. prospector will be ran only over those files you just changed for the current commit, no the whole project (as it ran by CI). So, it shouldn't be too slow.

On the other hand, each plugin of pre-commit can be disabled for a specific commit by:

SKIP=prospector git commit

and this command will run all of the other plugins but prospector.

BTW, it's still just an idea since I haven't tried too much yet... but I think it could be good to have the same info on your computer instead of waiting for CI to finish.

@ericholscher
Copy link
Member

@ericholscher ericholscher commented Nov 28, 2017

Neat, I like that it only applies to changed files.

@agjohnson
Copy link
Contributor

@agjohnson agjohnson commented Dec 1, 2017

Neat! This is a step that should be handled before commits go up anyways, but we'll see how intrusive pre-commit and plugins become as our linting efforts mature.

Errors look unrelated, merging this.

@agjohnson agjohnson merged commit ab30efe into master Dec 1, 2017
1 of 2 checks passed
@agjohnson agjohnson deleted the humitos/style/pre-commit-prospector branch Dec 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants