Hi,
Wiadomość napisana przez Stephen Finucane <sfinucan@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 :)
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@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