Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --black-enable option to turn Black on and off #8672

Closed
wants to merge 8 commits into from

Conversation

Eric-Arellano
Copy link
Contributor

NOTE: this is made on top of #8671. See Eric-Arellano/pants@refactor-black...Eric-Arellano:black-enable for the difference between the two.

Problem

We need a mechanism for users to say that they want to use a tool like Black or not. Not every codebase will want to use the tool, such as Pants not yet using it, but users should still be able to do ./pants fmt-v2 and ./pants lint-v2.

We currently do this in V1 through the --skip option, which is applied recursively so allows for --fmt-skip, --fmt-black-skip, --fmt-isort-skip, etc. This will not work with V2, however, because we cannot have options on specific "tasks" anymore. Options must either reside at the Goal level or in a subsystem.

Solution

Define the skip/enable mechanism directly on the tool itself, i.e. --no-black-enable and --black-enable.

This has a major benefit that in V2 we are setting up tools to provide both fmt and lint implementations, where possible. This means that Black will run under both ./pants fmt and ./pants lint. Were we to keep the V1 --skip implementation, we would need to configure:

[fmt.black]
skip: True

[lint.black]
skip: True

This rarely seems to be desirable. Presumably, if one is autoformatting with Black, they would also want to enforce that this is actually happening. (Note that discussion is irrelevant for linters like Pylint).

Instead, this approach looks like:

[black]
enable: True

Result

We can now write new rules for fmt-v2 and lint-v2, like implementing isort, without having those commands trigger Black when Black is not used by the codebase.

@Eric-Arellano
Copy link
Contributor Author

Closing in favor of the plugin system proposed in #8346 (comment).

@Eric-Arellano Eric-Arellano deleted the black-enable branch November 21, 2019 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant