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

Black integration, fixes #1662 #1672

Merged
merged 4 commits into from
Apr 9, 2024
Merged

Conversation

ThomasWaldmann
Copy link
Member

@ThomasWaldmann ThomasWaldmann commented Apr 5, 2024

@ThomasWaldmann
Copy link
Member Author

ThomasWaldmann commented Apr 5, 2024

@RogerHaase @UlrichB22 See the ran black on all python code commit about how it would look like.

@ThomasWaldmann
Copy link
Member Author

BTW, we should decide about this rather quickly - this PR is likely conflicting with other PRs a lot.

@UlrichB22
Copy link
Collaborator

IMO the line-length should be set to about 120. It looks like you already specified it, but I did not find the config.
For me it is ok to integrate black.

@ThomasWaldmann
Copy link
Member Author

@UlrichB22 It's there: d6e2137

@ThomasWaldmann
Copy link
Member Author

@RogerHaase can you have a look?

if there are no objections to the black style, i'ld like to merge this (and also add pre-commit integration of it).

@RogerHaase
Copy link
Member

Looks OK to me.

@ThomasWaldmann
Copy link
Member Author

rebased onto current master.

@ThomasWaldmann
Copy link
Member Author

ThomasWaldmann commented Apr 9, 2024

@RogerHaase @UlrichB22 please run this in your workdir after this is merged into master and you updated your local master from github:

# additionally install pre-commit:
pip install -r requirements.d/development.txt

# add the pre-commit hook, invoking black and flake8:
pre-commit install

If e.g. black has to auto-format something in the pre-commit phase, it aborts the commit.

Then you can have a look at the changes (if you like), git add these changes and try committing again (which then should succeed).

... and other parts of moin import it from here,
thus we suppress false positive "unused import" warnings.
@ThomasWaldmann
Copy link
Member Author

squashed together some of the commits. ^^^

@ThomasWaldmann ThomasWaldmann merged commit a2b0abd into moinwiki:master Apr 9, 2024
6 checks passed
@ThomasWaldmann ThomasWaldmann deleted the black branch April 9, 2024 17:22
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.

3 participants