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

Suggestion for Issue 401 #403

Merged
merged 5 commits into from
Feb 1, 2021
Merged

Conversation

Christovis
Copy link
Collaborator

@Christovis Christovis commented Jan 20, 2021

This PR will add a pre-commit routine as outlined here. The effect will be, that whenever we execute git commit -m ... black and flake8 will check and format the code automatically.

The configuration is such that the black formatting keeps the PEP8 line-length but there are some other very small differences (I don't know the details). If black really causes headaches, it can always be forced to be more faithful to PEP8.

I tested the pre-commit routine on ./bigbang/mailman.py. Thus there changes in that file are mainly because of black and because flak8 raised some warnings and errors.

New files:
./pre-commit-config.yaml
./pyproject.toml
./.flake8

@sbenthall
Copy link
Collaborator

I'm 👍 on this.

How do you feel about it @npdoty ? I feel like this sort of change should have your buy in!

@Christovis
Copy link
Collaborator Author

Christovis commented Jan 29, 2021

I have included isort into this PR, as it is another handy pre-commit routine that automates the sorting of package imports to make the order consistent across files.

Also, after you have reviewed the changes due to the pre-commit routine in mailman.py don't accept this PR straight away. Let's apply the routine on all files first.

@sbenthall
Copy link
Collaborator

Since @npdoty is in dissertation isolation mode, I will unilaterally approve this PR. Thanks for the contribution @Christovis

@sbenthall sbenthall merged commit 1473d08 into datactive:master Feb 1, 2021
@Christovis Christovis mentioned this pull request Feb 3, 2021
@Christovis Christovis deleted the issue-401 branch February 26, 2021 10:56
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.

2 participants