Conversation
Tests are currently failing because of PEP 8 issues. This patch fixes them and flake8 is now passing. Reformatting has been done using "black" automated tool.
48259cc to
a516665
Compare
|
Note: I have reformatted only the files causing trouble, and disabled string normalization, this is to avoid modifying too many files, and breaking other pull requests. |
|
Thank you very much. Well it would at least conflict with #1412 from @inclement I suppose 😅 |
|
@AndreMiras it should not be such a big deal to rebase after this PR, the lines are not modified a lot. |
|
I'd rather not merge this without having an overall decision of whether we want to use black on everything, or not. Some of black's code style choices are quite distinctive and different to what p4a tends to do at the moment. Personally, I'm not sure I'm a fan of some of these choices. In particular, black optimises for future patch simplicity over current code vertical conciseness, it is very eager to move brackets to new lines on their own even where this makes the code much taller than really necessary. I'm not dismissing black out of hand, in fact I've raised the possibility of using it more than once myself on irc/discord, I'd just rather not half use it this one time to generate a lot of code line changes that may not get maintained and aren't consistent with the rest of the code base. (Also this diff reveals some minor problems that would be nice to fix manually if we do run black, like the way it changes things like |
|
@inclement it makes sense. Anyway in a previous PR which has been merged, the style issues blocking the tests have been ignored, so tests are running again, and this was my main goal. So no problem for me if you want to close this PR and discuss style changes more extensively. I also think if an automated tool/code formatting is done, it should be done on the whole code base, and not just the few files causing trouble, like I did here. |
|
Okay, I'll close this PR for that reason. Thanks for making it though, and this doesn't mean we can't discuss using something like black. |
Tests are currently failing because of PEP 8 issues. This patch fixes
them and flake8 is now passing.
Reformatting has been done using "black" automated tool.