Skip to content

Do not mention redundant use case for Label constructor#14609

Closed
fmeum wants to merge 1 commit intobazelbuild:masterfrom
CodeIntelligenceTesting:fix-label-docs
Closed

Do not mention redundant use case for Label constructor#14609
fmeum wants to merge 1 commit intobazelbuild:masterfrom
CodeIntelligenceTesting:fix-label-docs

Conversation

@fmeum
Copy link
Copy Markdown
Collaborator

@fmeum fmeum commented Jan 20, 2022

Label strings passed to the default argument of attr.label() are resolved to a Label immediately relative to the call site of
attr.label(), so wrapping the argument with Label(...) is entirely redundant.

More context at:
bazelbuild/bazel-central-registry#63 (comment)

Label strings passed to the `default` argument of `attr.label()` are
resolved to a `Label` immediately relative to the call site of
`attr.label()`, so wrapping the argument with `Label(...)` is entirely
redundant.

More context at:
bazelbuild/bazel-central-registry#63 (comment)
@fmeum
Copy link
Copy Markdown
Collaborator Author

fmeum commented Jan 20, 2022

@Wyverald This part of the docs has always confused me. Could you maybe give this particular page a read and verify that everything is up-to-date now that Label will experience its rise to fame with bzlmod?

@Wyverald Wyverald self-requested a review January 20, 2022 17:03
@Wyverald
Copy link
Copy Markdown
Member

Thanks, this looks good -- but yeah I should really rewrite this maybe. It took myself a long time to understand all the intricacies of label semantics.

@fmeum
Copy link
Copy Markdown
Collaborator Author

fmeum commented Jan 20, 2022

Thanks, this looks good -- but yeah I should really rewrite this maybe. It took myself a long time to understand all the intricacies of label semantics.

This could also resolve #14503 in one go.

@fmeum
Copy link
Copy Markdown
Collaborator Author

fmeum commented Jul 6, 2022

@Wyverald Do you want to open an issue about updating the docs for Label? I would then close this PR, which I guess only covers a very small part of the required overhaul.

@Wyverald
Copy link
Copy Markdown
Member

Wyverald commented Jul 6, 2022

Filed #15821. Thanks!

@fmeum fmeum closed this Jul 6, 2022
@ShreeM01 ShreeM01 removed the awaiting-review PR is awaiting review from an assigned reviewer label Sep 15, 2022
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.

5 participants