cookiecutter-adafruit-circuitpython icon indicating copy to clipboard operation
cookiecutter-adafruit-circuitpython copied to clipboard

Published 20 hours ago • verified publisher icon adafruit

Double check black config against MicroPython

Open tannewt opened this issue 5 years ago • 14 comments

They run it with --line-length 99: https://github.com/micropython/micropython/pull/5700/files#diff-022e22ed1ca3d4ac963996318c4cd5e1R174

tannewt avatar Mar 03 '20 19:03 tannewt

@dherrada take a gander when you have a few minutes

siddacious avatar Apr 01 '20 00:04 siddacious

@tannewt, @siddacious and I were just discussing this and I think that there is a very valid reason to add this. Because pylint sets the line length limit at 100, it would make sense for black to do the same. Sorry for such a late response. I'm rather bad at keeping track of my github notifications

evaherrada avatar Apr 20 '20 20:04 evaherrada

@dherrada Can we automate it? I wouldn't do it by hand. I don't think its that worth it.

tannewt avatar Apr 20 '20 23:04 tannewt

@tannewt Yeah, I believe @sommersoft has got a pretty good workflow for patching the build.yml.

evaherrada avatar Apr 20 '20 23:04 evaherrada

Wouldn't this change requiring patching all Python files? Shouldn't we re-wrap everything if we change the line length?

tannewt avatar Apr 21 '20 17:04 tannewt

@tannewt I was thinking that we shouldn't re-wrap lines already wrapped at 88 characters, since that would be hard to do any other way than by hand, but that it would make sense for it to wrap at 99 in the future

evaherrada avatar Apr 21 '20 17:04 evaherrada

Will the black check complain if we change the config without changing the files?

tannewt avatar Apr 22 '20 18:04 tannewt

I don't think so, but I can definitely test this in a few libraries.

evaherrada avatar Apr 23 '20 17:04 evaherrada

@dherrada where is this at?

siddacious avatar Jun 05 '20 23:06 siddacious

@siddacious thanks for reminding me. I'll take a look today

evaherrada avatar Jun 08 '20 13:06 evaherrada

@tannewt Yep, black will complain if we do that. However, I think since all the black disables that need to be added have already been added, and every PR merged in the last few months has been formatted by black, we should actually be pretty safe to run the updated black directive in an automated fashion

evaherrada avatar Jun 08 '20 16:06 evaherrada

@tannewt Oh, just realized I should also mention that black will change things it turned into multiline things to single line things if it is now allowed. Essentially, anything in-between 88 and 99 characters that was turned into a multi-line thing will now be in a single line.

evaherrada avatar Jun 08 '20 16:06 evaherrada

@dherrada Sounds like a plan. Happy to see it updated even if a few things change.

tannewt avatar Jun 08 '20 21:06 tannewt

@sommersoft Would there be any way to actually run black (not black --check) on all the libraries and then have it change the directive using adabot?

evaherrada avatar Jun 22 '20 15:06 evaherrada