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

Use black as code formatter #188

Merged
merged 2 commits into from
Dec 1, 2023
Merged

Use black as code formatter #188

merged 2 commits into from
Dec 1, 2023

Conversation

whot
Copy link
Contributor

@whot whot commented Dec 1, 2023

Integrated into pyproject.toml and tests/test_code.py so we can enforce the formatting in the CI.

Fixes #187


Quick! let's do this before someone has an idea of refactoring everything :)

@whot
Copy link
Contributor Author

whot commented Dec 1, 2023

For easier review: manual changes in pyproject.toml, tests/test_code.py (to run black) and tests/run-fedora (to install black), rest is whatever black generated though I had to fix a small number of things like b"foo " b"bar" one-liners that for some reason didn't resolve to b"foo bar".

Copy link
Owner

@martinpitt martinpitt left a comment

Choose a reason for hiding this comment

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

Thanks! This looks 95% good, just needs a little manual work for the argparse code.

tests/test_api.py Outdated Show resolved Hide resolved
dbusmock/__main__.py Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
@martinpitt
Copy link
Owner

martinpitt commented Dec 1, 2023

pylint failure:

 dbusmock/templates/networkmanager.py:593:47: W0631: Using possibly undefined loop variable 'ap_path' (undefined-loop-variable)

That's because black broke the line, and the # pylint: disable needs to be on the same line. tools fighting other tools!

Integrated into pyproject.toml and tests/test_code.py so we can enforce
the formatting in the CI.

Fixes martinpitt#187
@whot
Copy link
Contributor Author

whot commented Dec 1, 2023

fixed the pylint error with a follow-up to make it unnecessary

martinpitt
martinpitt previously approved these changes Dec 1, 2023
Copy link
Owner

@martinpitt martinpitt left a comment

Choose a reason for hiding this comment

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

Cheers! Assuming green, this looks good. I'll change the max line length for the other tools as a follow-up.

@martinpitt
Copy link
Owner

Oh, one more black error

@whot
Copy link
Contributor Author

whot commented Dec 1, 2023

I'll change the max line length for the other tools as a follow-up.

arguably we should leave them as-is. Black will format the lines to 118 and enforce that, if you leave pylint/ruff on 130 (or remove the line length) then we can format with a trailing # fmt: off without having those two tools complaining about it (happened to me in the earlier 130 version)

Oh, one more black error

gah. fixed now (friday evening here, I probably shouldn't be pushing things :)

Copy link
Owner

@martinpitt martinpitt left a comment

Choose a reason for hiding this comment

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

Cheers! Green 🍀 is the new black! 🏴

@martinpitt martinpitt merged commit a382014 into martinpitt:main Dec 1, 2023
20 checks passed
@martinpitt
Copy link
Owner

Now off to enabling black in vim 😁

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.

Use black (or blue) for code style?
2 participants