Proposal: Remove black from CI pipeline
After working with
black in the CI pipeline for a couple of months, I am increasingly skeptical of the value it is adding, and have encountered some situations where it has introduced unnecessary complications. My reasons are as follows:
The vast majority of the formatting changes that
blackmakes are inconsequential and have no discernible impact on code functionality, quality or readability.
We have a pipeline fail here: https://git.gfz-potsdam.de/gweather/shakyground2/-/jobs/69384 because
flake8are contradicting each other.
blackinsists that the following line should be formatted as:
tag = tag[tag.index("}") + 1 :]
flake8 insists that it must be:
tag = tag[tag.index("}") + 1:]
This absolutely trivial issue is preventing a merge request from passing the CI pipeline - requiring time invested to find the solution.
I have also encountered problems myself using
black with type hinting, which prefers to change the perfectly valid
Union[x, None], which will introduce an error if
Union has not been imported. Again, it is not an insurmountable problem, but it is an unnecessary one.
Other differences such as line-length requirements can be configured to avoid conflicts (as they are currently), but I can see circumstances where a contributor is unfamiliar with
blackand may run the standard command
black ...with the defaults, only to have it modify the code in a way that is incompatible with the black configuration in the CI. You can see this particular error from me in the commits to this MR: !13 (commits), and though this might have been my own error it did cost time to understand and fix the problem, but for no gain in terms of code quality. For an outside contributor, annoyances like this can be a barrier to engaging with a project.
Quite simply, while the CI pipeline runs
blackis redundant for actually catching problems of formatting and readability. I am not suggesting to abandon the use of
blackentirely. As we have now begun to develop the code with one particular style I will continue to run
blackprior to submitting MRs and add this into the readme as an instruction to contributors to do the same. However, making
blackan integral part of the CI pipeline to the extent that trivial formatting issues causing it to break is adding complications for no gain.
@marius @rizac @ds I would not remove this without agreement (though I don't know how to get around the problem in point #2 (closed)), but I would like to put forward my views and experiences on this, which may be relevant to you if you are using
black in other projects.