Open Stack

On Thu, 2019-04-18 at 18:28 +0200, Dmitry Tantsur wrote:
> On 4/18/19 6:20 PM, Clark Boylan wrote:
> > On Thu, Apr 18, 2019, at 9:09 AM, Lance Bragstad wrote:
> > > 
> > > 
> > > On 4/18/19 9:00 AM, Monty Taylor wrote:
> > > > 
> > > > 
> > > > On 4/18/19 1:45 PM, Sean McGinnis wrote:
> > > > > > I think we already use flake8 openstack-wide, what will Black bring
> > > > > > us in
> > > > > > addition to that?
> > > > > > 
> > > > > > Dmitry
> > > > > 
> > > > > Based on my quick test, it would bring us code churn of changing ' to
> > > > > " and
> > > > > flake8 errors due to unwrapping lines and making them longer than 80
> > > > > characters.
> > > > > 
> > > > > I like the idea of code consistency. But having a tool automatically
> > > > > do it
> > > > > without being able to tweak the settings, and having defaults do
> > > > > things that
> > > > > introduce pep8 errors, seems like a non-starter.
> > > > > 
> > > > > And as mentioned elsewhere, introducing something like this to a long
> > > > > established code base has a lot of problems. Maybe new projects just
> > > > > starting
> > > > > off may want to investigate further, but running on most projects
> > > > > could be a
> > > > > nightmare.
> > > > 
> > > > I agree.
> > > > 
> > > > I actually looked at black a little while ago for openstacksdk -
> > > > because I do like me some strict style rules. The long-line-length
> > > > sucked, although making a tox env that passes a reasonable line-length
> > > > worked around that.
> > > > 
> > > > The biggest problem is that, even in a pretty clean codebase that
> > > > already tries to follow a lot of what black likes (line breaks after
> > > > brackets - yes please!) - the churn was MASSIVE and really just not
> > > > worth it.
> > > > 
> > > > If we were starting OpenStack from-scratch today, I'd recommend we
> > > > adopt it. But we're not.
> > > > 
> > > > Also - if new projects decided to adopt black, we'd wind up with two
> > > > conflicting codestyles across OpenStack, which is one of the things
> > > > our long-standing gating on pep8 is supposed to prevent. I don't think
> > > > it's a good idea for OpenStack to _partially_ adopt black.
> > > 
> > > ++
> > > 
> > > Conflicting code styles is just going to raise the cognitive bar for
> > > contributors. If black's conventions are too restrictive, but we still
> > > want more consistency, someone can drive an effort to start writing down
> > > conventions we, as in OpenStack, think are important. Maybe our desired
> > > conventions are closer to black's than we originally anticipated. Maybe
> > > they're not.
> > > 
> > > Regardless, it seems the underlying issue is applying any convention to
> > > a large codebase.
> > 
> > I've not seen it mentioned in this thread yet, but I think it is a fairly important point: Black's conventions are
> > rigid because Black aims to reduce merge conflicts and aid in code review by reducing diff sizes. The rules aren't
> > arbitrary. I think if we were to use Black the aim would be to get these benefits and accept that it may not always
> > format the code the way I (or you) want it.
> > 
> > Also, I don't think we would continue to follow pep8 if we switched to Black. It is one or the other.
> 
> Won't it make OpenStack code more surprising for outside contributors? I think 
> if something is worth following, it is PEP-8, since it's a Python standard.
pep8 is a set of guidelines that are intened for developers to use themsevels
to improve the readablity of there own code. it was never intended to be something
to force on others and it very clearly give peopel the fredom to not follow all of it.
https://www.python.org/dev/peps/pep-0008/#a-foolish-consistency-is-the-hobgoblin-of-little-minds

having some divergence might be unfamilar but we already diverge today.
also pep8 leave several ways to do some things

def func(arg1, arg2, arg3):
    pass


def func(arg1, 
         arg2,
         arg3):
    pass

def func(
    arg1, arg2, arg3):
    pass

all 3 of those are pep8 complient functions

black say the last two are dumb becasue you are wasting virticl space making diffs bigger
and will handel the wraping as need to also stay withind the default of 88 charaters.
you can override it to 79 if we wanted too  but even pep8 says up to 99 charater is fine
if that is what teams agree too.
https://www.python.org/dev/peps/pep-0008/#maximum-line-length

> 

That said I agree there would be a fairly high cost to convert an existing code base to it and I also agree we
probably don't want to have inconsistent rules across repos.

Another concern is that Black does apparently tweak rules over time which can lead to large sets of changes when
Black updates. We have similar problems with pep8 tooling, but we can do fine grained rule ignore lists and avoid
needing to incur massive churn all at once.

This can become a big issue, actually. We'd probably have to pin Black to an 
exact version (excluding it from global-requirements).


Clark