Deprecate FmtTaskMixin's --transitive option #8345
Assignees
Comments
|
No strong reason AFAICR, seems fine to me. |
Eric-Arellano
added a commit
that referenced
this issue
Nov 21, 2019
… preparation for switching default to `--no-{fmt,lint}-transitive` (#8666)
### Problem
Will close #8345.
These options cause tools like isort and Scalafmt to work on the transitive dependencies of the targets you specify, rather than only the targets specified. This is surprising and not how the tools work when called directly—you'd expect isort to only change the files you pass to it, for example.
We decided when adding this option to the V2 rules (#8660) that instead we should deprecate this misfeature.
However, we cannot simply deprecate the option in one fell-swoop because then people who are trying to prepare for the default behavior changing to `--no-transitive` will be met with a deprecation warning that the option will be removed. Leaving off the option so that there's no deprecation warning means that they will have a breaking behavior change in 1.25.0.dev2 when we no longer act transitively.
### Solution
For this deprecation cycle, only warn that the default will change if they are currently relying on the default.
In 1.25.0.dev2, after the default changes, _then_ we can safely deprecate the option outright.
### Result
Users who specified the option get this warning message:
> [WARN] /Users/eric/DocsLocal/code/projects/pants/src/python/pants/task/task.py:265: DeprecationWarning: DEPRECATED: Pants defaulting to --fmt-transitive and --lint-transitive will be removed in version 1.25.0.dev2.
Pants will soon default to --no-fmt-transitive and --no-lint-transitive. Currently, Pants defaults to `--fmt-transitive` and `--lint-transitive`, which means that tools like isort and Scalafmt will work on transitive dependencies as well. This behavior is unexpected. Normally when running tools like isort, you'd expect them to only work on the files you specify.
>
> To prepare, please add to your `pants.ini` under both the `fmt` and the `lint` sections the option `transitive: False`. If you want to keep the default, use `True`, although the option will be removed in Pants 1.27.0.dev2
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Goal-level options mixins don't work well with v2, because we don't have
Tasks to pass the options down to.It probably makes sense for
--transitiveto just be a Goal-level option; I can't think of use-cases for wanting different languages formatted with different levels of transitivity. If there are valid use-cases, we can work out some other replacement, but I quite like the simplicity of this one :)See also context from https://pantsbuild.slack.com/archives/C0D7TNJHL/p1569513933176700
@benjyw IIRC you added this - is there a strong reason for
--transitivenot to just apply to allfmtTasks?(I will file a separate ticket for
--skip, which I think should be handled differently)/cc @stuhood
The text was updated successfully, but these errors were encountered: