'The Big One': A large, repository-wide formatting commit coming in ONE WEEK

[Serene-Arc]

Hello all!

The 'What'

This is a warning for a big, yet simple change that will be coming to the beets repository in one week. Developers should take note, but this won't impact users at all.

When beets was started, tools for formatting code, especially for Python 3, were still in their infancy. As they progressed, the minimum version that they supported crept higher due to changes in the language, and what features they could support. Beets, trying to keep the range of compatible Python versions as wide as possible, has only recently gone above the minimum version supported by one of the most popular Python formatting tools, black.

Black is a tool based on the famous quote from Henry Ford: 'Any colour, as along as it's black.' Black is a python formatter that has very little in the way of customisation. You set it and it does its thing. No changes, no variations, no fiddly formatting options.

The Impact

This is what we're bringing into beets. In one week, black will be applied to the entire repository, along with the tool isort, which applies just to import statements. This commit will change nearly every file in the repository, and nearly every line as well. For developers, this means that there will suddenly be merge conflicts, and a lot of them.

Unfortunately, this is unavoidable. There will never be a time when there are no pending PRs in the beets repository. It's a popular tool and the maintainers are volunteers. If we're going to do this, it might as well be now.

The Benefits

Moving forward, this will make all merges easier to manage, since everything will be formatted to a standard and there will be no idiomatic formatting. This will make it easier for us maintainers to evaluate, read through, and resolve merge commits when dealing with pull requests, and it will make the code more readable.

Once the 'big commit' is done, it will never need to be done again. Every PR will be held to the black standard and will be unable to be merged unless all of the formatting checks out. Luckily, there are automations for this!

What Developers Can Do

Firstly, check out #4654 for the details on the commit that is coming and the changes.

Secondly, install pre-commit! This handy tool adds hooks to the git commit process, which are checks that have to pass before you can actually do the commit. When the 'big one' is merged to development, it will activate, requiring that every new commit conform to the standard.

You can also use the tox command tox -e format to manually run the formatter on all files. Since this is what we'll be doing to make the commit, it'll give you a good look at how the repository will look once it's done. Workflows will ensure that this formatting will have to apply to any PRs so use a combination of pre-commit and tox to make sure that your code matches.

TLDR

We're applying the black formatter to the entire codebase. This will make a great many merge conflicts so here's your warning for that.

If you have any questions, ask them here!

[sampsyo]

Thank you for wrangling this! And for the clear instructions for everybody.

[RollingStar]

Thanks for the work on this. Some requests:

1. Make an issue in Github for maximum visibility
2. Check in with frequent contributors, possibly in Zulip. I would like a week or two to check any outstanding PRs or commits I've made.
3. I think it's a huge mistake to commit this before Importer UI overhaul rebase/update #3721 is committed. That has been on and off for 7 years and it shouldn't be delayed any further.
4. Change the name of Add configurations and support for auto formatting tools #4654 to sound more dire. Or split into two PRs: "support" for Black and "Important: mandating" Black. Based on your description it will be at the very least an annoyance to deal with if you have half-finished PRs when the change goes live. The title should reflect that it is a semi-breaking change.

I support Black (I learned about it from those old discussions on Beets, actually) and use it in a good number of projects. I think it's the right move but with a little more effort we can make the transition more graceful. Beets isn't that active and putting out a last call for commits should help smooth things over.

Would it make sense to provide dev instructions to use Black on their commits before the hard changeover? This change should be easier if all the devs are already submitting Black-formatted PRs.

[Serene-Arc]

I'm deferring to @sampsyo on the timeline for this; if he thinks we should give it longer than a week, then we can give it longer than a week. The idea for this is to be the week that outstanding PRs get resolved or they get pushed until after the rebase. He's got the final call on that.

I'm leaving it as one PR. It's not possible to separate them since it includes workflows and configuration tools. If we split it up, then they'd need to be merged at the same time anyway since the workflows will prohibit any merges that don't meet the format requirements. This change needs to be applied as a cohesive change to workflows, both on GitHub and for devs.

I don't think it would make sense to provide the dev instructions. The tools we're using, like most formatters, don't apply to parts of a file; they'll apply to the entire file at minimum. If we tell developers to use the formatting tools before the commit, we'll have a dozen major formatting commits instead of one, possible a big commit for every file that people are working on. It's easier and makes more sense just to apply the formatting to the entire codebase and then let the individual PRs sort out the merge conflicts that arise, if any.

[JOJ0]

@Serene-Arc I'd also like to finally finish #3721 before this huge change. We are so close to getting that GUI. I just didn't have time to do the tiny changes due to personal / study reasons. Sorry for that! Could you wait another 2 weeks or something? Thanks so much!

[Serene-Arc]

Of course, and I know @sampsyo wants it done before the 'big one' as well. We'll definitely wait for it but people should know it's the exception. As discussed in the PR, there'll never be a time when there's no PRs waiting.

This is meant to be a big siren getting people ready; the week isn't a hard cut off for when we're going to make the commit but it will happen soon so if you'd like to avoid the trouble, try to get it done soon.

[sampsyo]

Maybe a useful route here would be to comment specifically on the #3721 thread to ask the folks involved whether they want to sneak in under the bar and merge before the reformat?

I am hopeful that, if the answer is "no," it won't be that hard to rebase: even though #3721 is a large changeset, updating it is hopefully "just" a matter of running Black with the appropriate config.