<div dir="ltr"><div dir="ltr"></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Apr 18, 2019 at 4:07 PM Monty Taylor <<a href="mailto:mordred@inaugust.com">mordred@inaugust.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
<br>
On 4/18/19 7:04 PM, Clint Byrum wrote:<br>
> Quoting Natal Ngétal (2019-04-18 01:51:47)<br>
>> Hi everyone,<br>
>><br>
>> Black is project to format Python code. It's more and more used by the python<br>
>> community. That can save time and harmonize the code. For example, simple quote<br>
>> versus double quote, that was not checked by pep8.i It's just an example of<br>
>> what make  black With a pre-hook commit hook this will be also avoid to lose<br>
>> time with pep8 errors.<br>
>><br>
>> Two links to see more:<br>
>><br>
>> <a href="https://github.com/ambv/black" rel="noreferrer" target="_blank">https://github.com/ambv/black</a><br>
>> <a href="https://www.mattlayman.com/blog/2018/python-code-black/" rel="noreferrer" target="_blank">https://www.mattlayman.com/blog/2018/python-code-black/</a><br>
>><br>
>> I think it can be very interesting to start to use Black on OpenStack. What<br>
>> do you thinks about that? For example, we can introduce it on some projects, I<br>
>> would volunteer to try on TripleO.<br>
> <br>
> I've read the arguments in the thread, and I really appreciate<br>
> everyone's thoughtfulness here.<br>
> <br>
> I just wanted to add that I have been using black for some internal<br>
> projects, and it is an absolute joy to forget about code formatting.<br>
> This is different than the pep8 checks. The git hook just does it, and<br>
> our CI confirms that you do it, and there's no more "oops I forgot<br>
> pep8".<br>
> <br>
> I believe that the whitespace churn that folks are bringing up is being<br>
> overblown. This is a single, automatically produced diff, that while it<br>
> might hurt a little in git annotate and conflicts with in-flight patches<br>
> now, there is value, and there are mitigations, that make it worth it in<br>
> some cases.<br>
> <br>
> The main value is that using an automatic formatter will ultimately lead<br>
> to less wasted test runs, less wasted developer time, and better diffs<br>
> with less conflicts later.<br>
> <br>
> So while I appreciate the desire to not rock the boat, it might be that<br>
> the boat would go faster after some dead weight is dropped overboard,<br>
> and that new energy is applied to forward progress instead of "darn it,<br>
> now I have to think about how to fit this into 80 chars and parenthesis<br>
> and ..." ><br>
> Basically, weigh the one time cost vs. the ongoing savings, rather than<br>
> considering them separately.<br>
<br>
I think it's useful to point fingers at concrete things if we're going <br>
to weigh costs vs savings.<br>
<br>
Here is a patch:<br>
<br>
<a href="https://review.openstack.org/653876" rel="noreferrer" target="_blank">https://review.openstack.org/653876</a><br>
<br>
That runs openstacksdk through black and adds it to the pep8 tox env. I <br>
had to make two manual code changes, and add two pep8 exclusions.<br>
<br>
I also did -l 79 - because it's important.<br>
<br>
I'd get so many stackalytics points for the +55540, -39832 :)<br>
<br>
Monty</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <br></blockquote><div><br></div><div>I think that the generated code is generally about as easy to read as the rest of the SDK project. That being said, it looks like black takes some of the guess work out of the picture. I'd support using it. Personally, I am a fan of the ~88 line length, but as long as black comes close to a consistent 79 (note  -l 79, according to the docs, does not guarantee no lines longer than 79, but that it will make an attempt as long as no other rules are violated) to make sure the folks who are more opinionated (hard -1/-2 for a line-length change) are happy with using black, it should be fine.</div><div><br></div><div>The change will be high-ish impact for backports (or require stable branches to receive the same treatment) and could impact in-flight patches. These are all things we've dealt with over and over in OpenStack.</div><div><br></div><div>Less guessing about code style and consistency is good in my opinion. It tends to lead towards an easier to maintain code base. I also *really* appreciate the simplicity of it being either a hook or baked into the pep8 check when run locally. I would advise that if black were to be run in the gate as part of the pep8 check, it should raise an error if the code changed between pre-black and post-black (change should always be done local to the developer, before hitting gerrit unless we can have hooks in gerrit to ensure consistency).</div><div><br></div><div>I vote monty get all the stackalytics points for all of the repos 1st run of black commit (or maybe the openstack proposal bot if assigning it all to Monty doesn't work.)</div><div><br></div><div>--Morgan</div></div></div>