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

Enforce black via pre-commit tool #53

Merged
merged 7 commits into from Jun 28, 2019
Merged

Enforce black via pre-commit tool #53

merged 7 commits into from Jun 28, 2019

Conversation

dwinston
Copy link
Contributor

For #33.

On git commit and git push, pre-commit runs black over relevant changed files. The first time pre-commit is run, it installs black in a separate, sandboxed Python environment that it uses on subsequent runs.

Add pre-commit install line to README, which installs hooks (from ./pre-commit-config.yaml) into .git/hooks/pre-commit so that e.g. black runs automatically on git commit.

@dwinston dwinston requested review from ml-evs and ltalirz June 24, 2019 23:54
@ml-evs
Copy link
Member

ml-evs commented Jun 25, 2019

Looks cool, I'll test tomorrow and review

Copy link
Member

@ltalirz ltalirz left a comment

Choose a reason for hiding this comment

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

thanks @dwinston !

One comment regarding python version, otherwise good.

By the way did you pre-commit run --all-files ?
Depending on how strict we want to be, you can also add something like pre-commit run --all-files || ( git status --short ; git diff ; exit 1 ) on travis

requirements.txt Outdated Show resolved Hide resolved
@dwinston dwinston requested review from ml-evs and ltalirz June 28, 2019 19:14
@dwinston
Copy link
Contributor Author

@ltalirz I also just added a commit with the changes suggested by pre-commit run --all-files.

Copy link
Member

@ml-evs ml-evs left a comment

Choose a reason for hiding this comment

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

LGTM, agree that there's no reason not to move to Python 3.7 since we're using typing so heavily.

Copy link
Member

@ltalirz ltalirz left a comment

Choose a reason for hiding this comment

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

thanks @dwinston !

@ltalirz ltalirz merged commit ed699f7 into master Jun 28, 2019
@CasperWA CasperWA deleted the enforce-black branch November 8, 2019 14:36
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

3 participants