Open Stack

On Thu, 2019-04-18 at 13:02 +0200, Slawomir Kaplonski wrote:
> Hi,
> 
> > Wiadomość napisana przez Stephen Finucane <sfinucan at redhat.com> w dniu 18.04.2019, o godz. 12:33:
> > 
> > On Thu, 2019-04-18 at 11:56 +0200, Dmitry Tantsur wrote:
> > > On 4/18/19 11:52 AM, Sorin Sbarnea wrote:
> > > > Black is a python code formatter, something similar to the older
> > > > yapf.
> > > > 
> > > > I do like to avoid discussions about formatting in code reviews and
> > > > prefer to let the linter take care of what could count as
> > > > appointed.
> > > 
> > > That's what pep8 jobs are for. They cannot be replaced with a pre-
> > > commit hooks, because people who are mostly likely to screw up
> > > formatting are also most likely to not have it installed.
> > > 
> > > > My experience with yapf was far from idea as they decided to change
> > > > formatting in a major way in their last *minor* version update,
> > > > breaking havoc.
> > > > 
> > > > There is one reason why I did not even try black: it does require
> > > > python3.6 !!!
> > > > 
> > > > While I do have a huge range of python version installed on my own
> > > > dev machines, I cannot say the same about the average
> > > > developer/contributor.
> > > > 
> > > > Considering that on CentOS-7 you can only get python3.5... this
> > > > rest my case.
> > > 
> > > You're welcome: 
> > > https://github.com/dtantsur/config/blob/master/tasks/packages.yml#L56-L72 :)
> > 
> > You've also got tox and virtualenvs in general, but I don't think this
> > is a serious issue tbh...
> > 
> > > > I do not want to limit the number of platform for contributors on
> > > > any project, especially when we talk about one of the most
> > > > basic/early check a developer would have to run on any change they
> > > > make.
> > > > 
> > > > That is why -1 on black, and I guess it will be like this until we
> > > > have CentOS 8 released. On the other hand, about yapf use I have
> > > > mixed feelings.
> > > > 
> > > > I would rather prefer to see better adoption/coverage of: flake8,
> > > > ansible-lint and yamllint -- preferably orchestrated via pre-commit 
> > > > as it lowers the execution time and disk footprint (sharing/caching
> > > > identical linters between repositories) -- opposed to tox which
> > > > does not shared anything between repositories. Tripleo-quickstart
> > > > and tripleo-ci repositories are already using it and I did not hear
> > > > many complaints, other than having to explain that pre-commit is
> > > > does not require the use of a git hook.
> > > 
> > > I think we already use flake8 openstack-wide, what will Black bring
> > > us in addition to that?
> > 
> > I've seen this used with packages such as tox and warehouse (the new
> > code base for pypi.org). black is intended to be an opinionated code
> > formatter and exposes no knobs to the user, meaning it's very much
> > "their way or the highway". This makes it different from something like
> > yapf, which exposes all these knobs and therefore allows for arguments
> > about e.g. whether one should use single or double quotes. No
> > configuration knobs means no ability to discuss these preferences,
> > which for something as subjective as code formatting seems to be a good
> > thing to me.
> > 
> > Now, as to whether it's practical, you have to realize that for it to
> > be of any use, black has to be run initially over your entire codebase
> > and then rerun on each commit. The latter isn't an issue (we already
> > run flake8 for this purpose) but I genuinely cannot see something like
> > nova rewrapping every line of code (all 552,347 of them, if my quick
> > check is correct), essentially breaking the ability to cleanly backport
> > patches and get meaningful 'git blame' output simply to avoid the
> > occasional debate about code formatting. I think adopting black for a
> > _new_ project (or even a very small one) could be a great idea, but for
> > something as large and cumbersome as nova (or neutron, glance, cinder,
> > some of the oslo libraries, etc.), maaaaaybe not.
> 
> +100 to this. Resolving manually many conflicts between master and stable branches just because e.g. quoting was
> changed doesn’t seems like good thing :)
you realise if we also applied this to the stable branches too we would never need to resolve
a merge conflict due to formating again or correct any pep8/style issues on backports.
but yes its a valid concern.

i guess the general feedback so far has been unless there is a way to do it incrementally this is a non starter.
that is sad but if that is how people feel ill let this to the original poster to fight.
> 
> > 
> > Stephen
> > 
> > PS: As an aside, I would like to see something like black provided as
> > part of the Python standard library to settle formatting issues for
> > once and for all and help ensure that a minimum standard is set for
> > most code you'll find (though code quality covers far more than mere
> > formatting). To the best of my knowledge, the gofmt tool in go(lang)
> > fills this role but I really can't see that happening within Python any
> > time soon.
> > 
> > > Dmitry
> > > 
> > > > 
> > > > Shortly, +2 on more pre-commit use, the tool not the git hook.
> > > > 
> > > > 
> > > > PS. Please do not bring the SCL into the thread, they do not
> > > > provide a single command enablement of additional python
> > > > interpreters (not in PATH).
> > > > 
> > > > > On 18 Apr 2019, at 09:57, Dmitry Tantsur <dtantsur at redhat.com>
> > > > > wrote:
> > > > > 
> > > > > Hi,
> > > > > 
> > > > > On 4/18/19 10:51 AM, Natal Ngétal wrote:
> > > > > > 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.
> > > > > 
> > > > > I'm not sure what exactly Black does, but the last thing I want
> > > > > to see is patches changing between single and double quotes. If
> > > > > it's not the case or it can be turned off, I'm all for
> > > > > experimenting.
> > > > > 
> > > > > > 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.
> > > > > 
> > > > > Given it's a pre-commit hook, isn't it supposed to be done on
> > > > > developer's machines? If so, how do you see applying Black to the
> > > > > whole OpenStack?
> > > > > 
> > > > > Dmitry
> > > > > 
> > > > > > My best regards
> 
> — 
> Slawek Kaplonski
> Senior software engineer
> Red Hat
> 
>