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

doc: improve alias error msg in pip hubs #2458

Merged
merged 1 commit into from
Nov 30, 2024

Conversation

aignas
Copy link
Collaborator

@aignas aignas commented Nov 30, 2024

Summary:

  • Add more flag values that we actually use.
  • Point users to online docs.

I still think it is not good enough when somebody needs to debug, but I
am not sure what to add. Maybe we just need user documentation outlining
what each config setting means and what values it is using? Maybe we
should generate the config settings inside the rules_python repository
so that we have better visibility into what needs to be documented and
explained.

Summary:
* Add more flag values that we actually use.
* Point users to online docs.

I still think it is not good enough when somebody needs to debug, but I
am not sure what to add. Maybe we just need user documentation outlining
what each config setting means and what values it is using? Maybe we
should generate the config settings inside the `rules_python` repository
so that we have better visibility into what needs to be documented and
explained.
Copy link
Collaborator

@rickeylev rickeylev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One trick we could do is map default to a error rule. e.g.

... = select(default: ":unknown_config")
unknown_config(name = "unknown_config", template = """
Unknown python. Curretn version is {python_version}
"""
)

def unknown_config_impl(ctx):
  fail(ctx.attr.template.format(
    python_version = ctx.attr._python_version_flag[BuildInfo].value
  ))

This would allow putting the current python version into the message directly.

@rickeylev rickeylev added this pull request to the merge queue Nov 30, 2024
Merged via the queue into bazelbuild:main with commit 25aeb40 Nov 30, 2024
4 checks passed
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.

2 participants