search

kneasle / wheatley

An AI for Ringing Room that can ring any number of bells to increase the scope of practices.
https://pypi.org/project/wheatley/
MIT License
15 stars 13 forks source link

Reformat code with black #197

Closed kneasle closed 3 years ago

kneasle commented 3 years ago

I've been meaning to run a proper autoformatter on the code for a while, and this PR runs Black on all code files to standardise the format. I've also added a GH action to check incoming PRs for formatting, but this might cause more friction than it solves - I think it's optional.

kneasle commented 3 years ago

Does this need to be added to contributing.md? E.g a link to https://github.com/psf/black/blob/master/docs/editor_integration.md. Does it need to be a global install or should it be added to requirements.txt?

That's a good point - I installed it through pip so requirements.txt should work. I've added both now

Overall looks fine and more compliant with PEP8. There are a few places where I think the format's not as good (e.g. the rows of the singles test going all to one line) and some interesting trailing commas, but I understand why they don't want to make it configurable

Every formatter will make bad calls at some point - but I'd much rather have a consistent and sometimes weird formatting style over having the code be all hand-formatted. Also I think trailing commas in line separated argument lists are awesome - it means that swapping or adding arguments is always a trivial copy-paste job.

However, regarding the wacky formatting on the tests, I think there might be a way to disable the formatter on parts of files (rustfmt definitely has this, but that exploits some syntax that python doesn't have). It didn't seem worth fixing it IMO, since it isn't critical and doesn't affect the main code of Wheatley.

kneasle commented 3 years ago

Right - this has been merged (though the rogue build workflow was still there)... now the fun starts fixing any merge conflicts with my other feature branches :sweat_smile:.