Skip to content

GitLab

Open
Created by Graeme Weatherill@gweatherOwner

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:

  1. The vast majority of the formatting changes that black makes are inconsequential and have no discernible impact on code functionality, quality or readability.

  2. We have a pipeline fail here: https://git.gfz-potsdam.de/gweather/shakyground2/-/jobs/69384 because black and flake8 are contradicting each other. black insists that the following line should be formatted as:

tag = tag[tag.index("}") + 1 :]

while 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 Optional[x] to 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.

  1. 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 black and 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.

  2. Quite simply, while the CI pipeline runs flake8 then black is redundant for actually catching problems of formatting and readability. I am not suggesting to abandon the use of black entirely. As we have now begun to develop the code with one particular style I will continue to run black prior to submitting MRs and add this into the readme as an instruction to contributors to do the same. However, making black an 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.