On 4/19/19 1:05 AM, Monty Taylor wrote:
On 4/18/19 7:04 PM, Clint Byrum wrote:
Quoting Natal Ngétal (2019-04-18 01:51:47)
Hi everyone,
Black is project to format Python code. It's more and more used by the python community. That can save time and harmonize the code. For example, simple quote versus double quote, that was not checked by pep8.i It's just an example of what make black With a pre-hook commit hook this will be also avoid to lose time with pep8 errors.
Two links to see more:
https://github.com/ambv/black https://www.mattlayman.com/blog/2018/python-code-black/
I think it can be very interesting to start to use Black on OpenStack. What do you thinks about that? For example, we can introduce it on some projects, I would volunteer to try on TripleO.
I've read the arguments in the thread, and I really appreciate everyone's thoughtfulness here.
I just wanted to add that I have been using black for some internal projects, and it is an absolute joy to forget about code formatting. This is different than the pep8 checks. The git hook just does it, and our CI confirms that you do it, and there's no more "oops I forgot pep8".
I believe that the whitespace churn that folks are bringing up is being overblown. This is a single, automatically produced diff, that while it might hurt a little in git annotate and conflicts with in-flight patches now, there is value, and there are mitigations, that make it worth it in some cases.
The main value is that using an automatic formatter will ultimately lead to less wasted test runs, less wasted developer time, and better diffs with less conflicts later.
So while I appreciate the desire to not rock the boat, it might be that the boat would go faster after some dead weight is dropped overboard, and that new energy is applied to forward progress instead of "darn it, now I have to think about how to fit this into 80 chars and parenthesis and ..." > Basically, weigh the one time cost vs. the ongoing savings, rather than considering them separately.
I think it's useful to point fingers at concrete things if we're going to weigh costs vs savings.
Here is a patch:
https://review.openstack.org/653876
That runs openstacksdk through black and adds it to the pep8 tox env. I had to make two manual code changes, and add two pep8 exclusions.
I quite hate some of the changes it made, e.g. https://review.opendev.org/#/c/653876/1/openstack/baremetal/v1/_proxy.py@296 is a horrible waste of screen space, while https://review.opendev.org/#/c/653876/1/openstack/baremetal/v1/node.py@267 also looks pretty weird to me.
I also did -l 79 - because it's important.
I'd get so many stackalytics points for the +55540, -39832 :)
Monty