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

Consistent option hierarchy #20898

Open
grihabor opened this issue May 9, 2024 · 0 comments
Open

Consistent option hierarchy #20898

grihabor opened this issue May 9, 2024 · 0 comments

Comments

@grihabor
Copy link
Contributor

grihabor commented May 9, 2024

The problems

Right now Subsystem options, Goal options and Target fields are unrelated and therefore inconsistent.

This means that every backend has to define it's own options for all the levels and make sure they all work together nicely.

Problems with --filter

Simple filtering on cli works:

pants lint --filter-target-type=python_source ::

Now imagine you want to do the same thing in pants.toml, how do you do that? I would expect this to work:

[lint]
filter-target-type = ["python_source"]

But it doesn't, because lint goal doesn't have filter-target-type option, for some reason filter is a subsystem of it's own:

[filter]
target_type = ["python_source"]

This means you can't control individual goals filter options.

Problems with skip

Filtering linters to run happens on all 3 layers of options.

To choose one single linter in CLI you can use only option in lint goal subsystem:

pants lint --only=shfmt ::

To skip one single linter it has to implement skip option in it's own subsystem:

pants lint --shfmt-skip ::

To skip one single target the target itself has to implement skip field:

shell_sources(skip_shfmt=True)

Inconsistencies between options and fields

In general, there are many options that are present in subsystem, but missing in the target, and vise versa. For example, there is a verbosity option in pex subsystem, but the option is missing in pex_binary. On the other hand pex_binary has layout field that is missing in pex subsystem.

Environments

This inconsistency probably involves interaction with environments too.

The solution

Not sure how to fix it, but it will probably require redesigning a bunch of pieces including core into a sound option hierarchy. TODO

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant