format code via black - Githubissues

ev3dev / ev3dev-lang-python

Pure python bindings for ev3dev

MIT License

425 stars 144 forks source link

format code via black #713

Closed dwalton76 closed 4 years ago

dwalton76 commented 4 years ago

Hmm this is a very large patch...9200 lines

WasabiFan commented 4 years ago

Wow, yeah, that's pretty big!

Although I am rather fond of keeping style consistent and using these tools, I feel like in this case the sheer amount of stuff that would change makes it not worth it. It makes blames/diffs/history searching hard for a relatively small benefit. I think we should leave the formatting inconsistent unless there's another compelling reason to change so much.

dwalton76 commented 4 years ago

On the flip side...

if we are ever going to do it now (before we pull an ev3dev-buster branch) is the time
once we rip off this bandaid and make the style consistent we could use a precommit hook to keep it this way (make black --check pass in order for the commit to be allowed).

I could go either way but am leaning towards "lets do it and be done with it".

dlech commented 4 years ago

Or use yapf instead of black and stick with single quotes

dwalton76 commented 4 years ago

I hadn't heard of yapf but it looks worth a try. I'll post another PR using yapf and we can compare the diff for it vs black.

dwalton76 commented 4 years ago

For comparison the yapf patch is 4300 lines...so still significant https://github.com/ev3dev/ev3dev-lang-python/pull/716

WasabiFan commented 4 years ago

OK, I'm up for formatting. I do prefer the yapf results just because it modifies less code.

If that's the one we're going to go with, can you add it (asserting matching style) to the TravisCI scripts? It can probably go in the CPython-only build (as opposed to micropython).

dwalton76 commented 4 years ago

If that's the one we're going to go with, can you add it (asserting matching style) to the TravisCI scripts? It can probably go in the CPython-only build (as opposed to micropython).

We should be able to do a pre-commit hook that verifies the style is correct, that way the user can't even commit it if the style is wrong.

WasabiFan commented 4 years ago

I generally prefer to avoid commit hooks... Commits are free and easily squashable backups, and I don't want something like a style check to get in the way of that. Being able to quickly push half-baked changes then pull them from another machine and amend your work there is very useful. I'd also rather not have to fight cross-platform scripting issues.

dwalton76 commented 4 years ago

We formatted via yapf instead