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

Include name in repr of exported rules #18392

Closed
wants to merge 1 commit into from

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented May 14, 2023

For an exported rule foo_library, repr(foo_library) now evaluates to <rule foo_library>, not <rule>, matching the behavior of native rules more closely.

Fixes #17483

@fmeum fmeum changed the title Include rule class name in repr of rules Include name in repr of exported rules May 14, 2023
@fmeum fmeum marked this pull request as ready for review May 14, 2023 16:03
@fmeum fmeum requested a review from a team as a code owner May 14, 2023 16:03
@fmeum fmeum requested review from aiuto and removed request for a team May 14, 2023 16:03
@github-actions github-actions bot added awaiting-review PR is awaiting review from an assigned reviewer team-Configurability platforms, toolchains, cquery, select(), config transitions labels May 14, 2023
@fmeum
Copy link
Collaborator Author

fmeum commented May 14, 2023

@comius Could you review this (very small) change?

@fmeum fmeum requested review from comius and removed request for aiuto May 15, 2023 17:03
@fmeum
Copy link
Collaborator Author

fmeum commented Jun 6, 2023

@comius Friendly ping

@comius comius added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Jun 7, 2023
@comius
Copy link
Contributor

comius commented Jun 7, 2023

I'm slightly reluctant to approve this, but seems to be the best of the bad choices.

In general people shouldn't rely on string representation.

The PR fixes a previous unintentional change in the representation.

The representation has the problem of not including .bzl file where the rule was created. But that's the same problem that pests ctx.rule.kind.

@copybara-service copybara-service bot closed this in 002490b Jun 7, 2023
@Pavank1992 Pavank1992 removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Jun 7, 2023
amishra-u pushed a commit to amishra-u/bazel that referenced this pull request Jun 7, 2023
For an exported rule `foo_library`, `repr(foo_library)` now evaluates to `<rule foo_library>`, not `<rule>`, matching the behavior of native rules more closely.

Fixes bazelbuild#17483

Closes bazelbuild#18392.

PiperOrigin-RevId: 538458291
Change-Id: I331955655756a3c235be0a93f1394716ddf9849d
@fmeum fmeum deleted the rule-name branch June 8, 2023 22:21
@fmeum
Copy link
Collaborator Author

fmeum commented Aug 10, 2023

@bazel-io flag

@bazel-io bazel-io added the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Aug 10, 2023
@iancha1992
Copy link
Member

@bazel-io fork 6.4.0

@bazel-io bazel-io removed the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Aug 10, 2023
bazel-io pushed a commit to bazel-io/bazel that referenced this pull request Aug 10, 2023
For an exported rule `foo_library`, `repr(foo_library)` now evaluates to `<rule foo_library>`, not `<rule>`, matching the behavior of native rules more closely.

Fixes bazelbuild#17483

Closes bazelbuild#18392.

PiperOrigin-RevId: 538458291
Change-Id: I331955655756a3c235be0a93f1394716ddf9849d
iancha1992 pushed a commit that referenced this pull request Aug 22, 2023
For an exported rule `foo_library`, `repr(foo_library)` now evaluates to
`<rule foo_library>`, not `<rule>`, matching the behavior of native
rules more closely.

Fixes #17483

Closes #18392.

Commit
002490b

PiperOrigin-RevId: 538458291
Change-Id: I331955655756a3c235be0a93f1394716ddf9849d

Co-authored-by: Fabian Meumertzheim <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Configurability platforms, toolchains, cquery, select(), config transitions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

repr representation of starlarkified rules is too generic
5 participants