Not sure if I like the recommendation to not let Black change your code and just give out errors.
I absolutely let Black change code and see the value in Black that it does that so the devs do not have to spend time on manually formatting code.
Black shouldn't break anything (and hasn't broken anything for me in the years I used it) but in the unlikely case it does it, there's still pytests/unittests after that that should catch problems...
As I understood it, it was to not let black do the formatting during CI builds. In local dev you’d let it reformat.
Even while it won’t break anything you want CI to be your safety net, flagging a local setup as being wrong is more valuable than magically autocorrecting it.
> CI/CD has no business changing your code; it builds stuff using it, exactly as if commit such-and-such.
That going too far unless you define code to be a subset of the files checked into the repository and simply define any file that's touched in an automated manner to be not code
There are a lot of useful automations that can be part of the CI/CD pipeline, such as increasing a version number, generating a changelog, creating new deployment configuration etc
They don't have to be part of it and it's possible to work around it/don't commit... But that comes with it's own challenges and issues
It's not about not trusting automated generation of certain artifacts. Stamping version numbers may be a fair idea. (A changelog entry needs human review and approval, in my book.)
It's mostly about the flow of data and control: source files, beside some known auto-generated files / single lines, are the source, and whatever is generated is downstream from them, not altering them. It's like a React app: data flows through props in only one direction, you don't patch DOM in event handlers, or something.
Its completely fine if you wish to treat your repository like that, but that doesn't make this treatment the only viable and correct way.
There are a lot of people which include their helm package as part of their project repository and even more that generate the changelog from standardized git messages like conventional commits and still wish to persist them in a changelog.md inside of the repository to make completely banal examples spelled out to the letter. It works well and lets you scale these things pretty well inside of a corporation with a lot of teams.
Its completely fine to keep all that out of your automated pipeline, triggering only after these things have occured... but that's not the only viable choice a developer can make and you're very much talking from of an extremely limited perspective if thats your point of view.
This is the way. We have format-on-save in our editor, works like a charm. Sometimes the CI still catches sometimes, but generally its very low friction.
My current project is my first project in a while which does not use black.
I liked black, though I was never satisfied with the fact that there was no way to normalize quotes to be single quotes: '. Shift keys are hard on your hands, so avoiding " makes a lot of sense to me. But there's the -S option that simply doesn't normalize quotes so it has never been a real issue.
However, this new project has a lot of typer functions with fairly long parameter lists (which correspond to command line arguments so they can't be broken up).
black reformats these into these weird blocks of uneven code that are very hard to read, particularly if you have comments.
Everyone is a fan of black; no one liked the result. :-/
I have a key in my editor to blacken individual files, but we don't have it as part of our CI. Perhaps next project again.
> I absolutely let Black change code and see the value in Black that it does that so the devs do not have to spend time on manually formatting code
100% this. I also let Black auto-format code in the CI and commit these formats.
A lot of developers, intentionally or not, don't have commit hooks properly setup. If Black doesn't change the code in CI they need to spend another cycle manually fixing the issues that Black could have just fixed for them.
You're saying that there's a risk that Black could break your code when formatting? Well, so could developers and I'd trust a machine to be less error-prone.
I don't understand why people are against this so much. Black does a sanity check and compares the AST before and after to make sure there aren't any meaningful changes (unless you are running it with --fast). So there is almost no risk that it will break your code.
There is nothing more frustrating than coming back from a coffee break only to find out that you have to rerun your CI check because of a trivial formatting issue.
If you use feature branches, it is quite annoying that can't push two consecutive commits because CI changed something in your branch and now you have to resolve conflicts.
We have it set up to only run after you've opened a PR. You shouldn't run into the issue often that way because if you're opening a PR then all your big immediate changes are already committed so you won't be pushing another commit until someone reviews which will be after black has already finished.
Sometimes I still sneak in minor changes after I opened a PR. Sometimes I open PR early because CI has integration/e2e tests that are hard to run locally. Sometimes I want feedback on certain parts early on and PR is the easiest way to show something.
There can be workflows in which pushing through CI works fine, but as a general advice it's not great because there are many edge cases.
> Not sure if I like the recommendation to not let Black change your code and just give out errors.
Let black format code before it is checked in. Code should not be reformatted for CI or production, and bad formatting should either ALWAYS throw errors (no known defects allowed) or NEVER throw errors (if it passes tests & runs ship it). Consistency is the key.
I absolutely let Black change code and see the value in Black that it does that so the devs do not have to spend time on manually formatting code.
Black shouldn't break anything (and hasn't broken anything for me in the years I used it) but in the unlikely case it does it, there's still pytests/unittests after that that should catch problems...