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

docs: goal associated with multiple backends only lists one #17772

Open
huonw opened this issue Dec 10, 2022 · 6 comments
Open

docs: goal associated with multiple backends only lists one #17772

huonw opened this issue Dec 10, 2022 · 6 comments
Labels

Comments

@huonw
Copy link
Contributor

huonw commented Dec 10, 2022

Describe the bug
The export-codegen goal is associated with/created by many backends, like protobuf, docker, and several others. However, the docs page for it (https://www.pantsbuild.org/docs/reference-export-codegen) only lists one: pants.backend.docker.

image

Pants version
2.14

OS
N/A

Additional info
N/A

@huonw huonw added the bug label Dec 10, 2022
@huonw
Copy link
Contributor Author

huonw commented Dec 12, 2022

This seems to be somewhat of an explicit choice:

@staticmethod
def get_provider(providers: tuple[str, ...] | None) -> str:
if not providers:
return ""
# Pick the shortest backend name.
return sorted(providers, key=len)[0]

#13993

@kaos
Copy link
Member

kaos commented Dec 12, 2022

Yea, I found it counter-productive to list all backends that registered types/rules, as there is quite a bit of cross pollination going on, due to dependencies between backends. Listing them all would be rather confusing in terms of presenting "the source" for where it came from.

The idea here being, that it is reasonable to assume that the shorter module path is likely a "top level" thing that would make sense to enable in the [GLOBAL].backend_packages configuration.

@kaos
Copy link
Member

kaos commented Dec 12, 2022

Having said that, perhaps for goal_rules, the list of providers rarely is so long so it could make sense to not limit this to just one entry..?

@huonw
Copy link
Contributor Author

huonw commented Dec 12, 2022

Thanks for the insight. I tried the quick thing of switching to including all providers for all options (huonw@8b82d45#diff-6e1f224c2d6075bc35b2fe6023c3aef055e5985c7f4042676b0f9617e296c3af).

I checked in the docs so that the rest of that diff shows what changed. It looks like there's a few broad categories of what's changed (just a selection, this isn't a complete categorisation of all files):

  1. useless extra info along the lines of what you say:
    • 'core' features: generated-docs/GLOBAL.md, generated-docs/generate-lockfiles.md, generated-docs/pex.md, generated-docs/pex-cli.md
    • multiple backends that are all related (e.g. where there's one 'parent' one that would definitely be enabled if any of the others are): generated-docs/golang.md, generated-docs/kotlin-infer.md, generated-docs/shellcheck.md
    • weird cases: generated-docs/experimental-bsp.md, generated-docs/check.md
  2. cases where the backends aren't (all) subsets of each other, and so listing multiple is potentially useful: generated-docs/lambdex.md, generated-docs/jvm.md, generated-docs/jvm_artifact.md, generated-docs/docker.md, generated-docs/protoc.md (e.g. for the JVM ones, someone might reasonably activate pants.backend.experimental.scala without activating pants.backend.experimental.java, but the current docs show Java only)
  3. cases where pants.core appears but isn't the best choice: generated-docs/black.md, generated-docs/yapf.md
  4. a bit of all of the above: generated-docs/local_environment.md, generated-docs/docker_environment.md

This definitely seems like an annoying thing to get 'right'! Maybe the current behaviour is the best balance 😄

@kaos
Copy link
Member

kaos commented Dec 12, 2022

Haha, nice research. Yea, there are certainly cases where showing just one is not perfect or the shortest one is not what we'd like. As you say, getting this right for every case will be tricky I think.

@Eric-Arellano
Copy link
Contributor

Thanks @huonw! I agree with your classification. How about we allow GoalSubsystem authors to give a hint what behavior we want? We default to the current behavior of only one backend, whichever has the shortest name (so we use pants.backend.python rather than pants.backend.python.lint.isort). But you can set a class property to say that the docs and help should instead list every backend.

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

No branches or pull requests

3 participants