Adopt Black for Automatic Code Formatting

[dboitnot]

Adopt Black for Automatic Code Formatting

Per discussion in Slack, here's an issue to discuss adoption of the Black code formatter. This would provide a consistent and high-quality code-style without burdening developers. Integrations exist for most editors and for pre-commit.

[dboitnot]

Assuming we get consensus here, what would be the next step? Submitting a PR with all files reformatted? Or would we take a more phased approach?

[jfalkenstein]

I'm largely in favor of instrumenting with black... it's just the first time you run it there's going to be an insane number of changes applied. I've not yet found the ideal approach in situations like that. The best I've come across in the past is doing it one top-level directory at a time, which reduces PR size. Once you get we get all the codebase formatted, we can add it to our pre-commit config to enforce it in the future.

@ericabrauer @alexharv074 @zaro0508, What do you guys think of using Black for this purpose? ^^

[ericabrauer]

I'm down! I feel like this would be a good starter for me for getting familiar with the code to start reformatting testing!

[zaro0508]

I'm in favor of using black. IMHO, this is a band aide situation. In situations where i think we don't have enough automated testing in place I would lean towards a phased approach. However I think sceptre has an adequate amount of unit and integration tests so I would lean on just ripping it off all at once so that the discomfort is short-lived rather than removing a little at a time.

[jfalkenstein]

To be fair, I've yet to find a case where Black broke code.

[dboitnot]

For what it's worth, I've been using it for months and never seen it break anything.

[dboitnot]

So I'm preparing a PR for this and I ran into an issue with flake8:

flake8...................................................................Failed
- hook id: flake8
- exit code: 1

tests/test_cli/test_prune.py:50:52: E203 whitespace before ':'
tests/test_cli/test_launch.py:51:52: E203 whitespace before ':'

This is because black surrounds the colon in a list slice with spaces and flake8 doesn't like it. Here's an example of Black's change:

                for stack in all_stacks[start_index : start_index + 2]

The Black project folks reckon this is a false positive and recommend disabling W503 until this issue is resolved in the pycodestyle project.

I tend to agree, so how do we feel about disabling 203 in the commit hooks? I figure once Black's in the hooks it'll be moot anyway.

[zaro0508]

disabling 203 sounds fine to me @dboitnot

[dboitnot]

Note the recommendation from Black:

Disable W503 and enable the disabled-by-default counterpart W504.

I did not do this in the PR as it was not necessary to get the checks to pass. But I wanted to mention it here in case we felt like doing that.