Re: [BuildStream] [Development topic] Use black to format code



Hi Chandan,

I'm a little late to the party, but we've actually previously discussed
this on #buildstream - only rejecting the idea eventually because
backporting to 1.x will become even more difficult if we completely
change our formatting rules. If we make the switch we'll need to accept
that backporting will essentially involve re-writing a patch (although
we're probably at that point already).

That said, I'd personally find it a fair bit easier to work on
BuildStream code if it supported being formatted by black (no more
worrying about how I'm going to split up my 200 column line), so I'd
really like to see this. There definitely was interest from others in
the chat as well.

These are fair questions. I didn't talk about the tooling much in the initial
message as I was trying to gauge interest in the idea first. Here are my
thoughts on how this would work:

  * We provide 2 tox environments - format and format-check. The `format`
    environment will format the code using `black`. In future, if we add any
    other formatters (like isort), they could presumably be run in the same
    environment. The `format-check` environment should do similar things as the
    `format` environment, but instead of rewriting the files, it would just
    output what would have been changed.

  * We expect developers to submit already formatted code along with their
    patches. This could be done easily with something like `tox -e format`.

  * We add a new CI job (or modify an existing one) to run the `format-check`
    environment as a check, similar to how we currently run the linters.

If we follow something like above, we shouldn't need blanket reformatting
commits from an automation. Rather, our codebase would be formatted correctly
at all points in history.

I don't see any problem with this sort of setup, and it's fairly
commonly used to my knowledge (seems to be semi-standard with `rustfmt`
for Rust code, too). It also addresses both Tom and Darius' comments.

So, in a nutshell, yes please!

Tristan


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]