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.

Sign up for GitHub

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

Jump to bottom

Implement describeKey for more actions #21380

Closed
wants to merge 1 commit into from
Closed

Implement describeKey for more actions #21380

wants to merge 1 commit into from

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Feb 16, 2024 •
edited
Loading

No description provided.

@fmeum fmeum requested a review from a team as a code owner February 16, 2024 15:12
@fmeum fmeum requested review from gregestren and tjgq and removed request for a team and gregestren February 16, 2024 15:12
@github-actions github-actions bot added team-Configurability platforms, toolchains, cquery, select(), config transitions awaiting-review PR is awaiting review from an assigned reviewer labels Feb 16, 2024
tjgq
tjgq requested changes Feb 16, 2024
View reviewed changes
Copy link
Contributor

@tjgq tjgq left a comment

Choose a reason for hiding this comment

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

This will only cover ctx.actions.symlink when the output is a declare_symlink. Do you also want to add an implementation for SymlinkAction, which would also cover declare_file and declare_directory outputs?

@tjgq
Copy link
Contributor

tjgq commented Feb 16, 2024

This will only cover ctx.actions.symlink when the output is a declare_symlink. Do you also want to add an implementation for SymlinkAction, which would also cover declare_file and declare_directory outputs?

(Feel free to push back if this isn't something you currently need, I'm happy to import as-is.)

@fmeum fmeum force-pushed the symlink-describe-key branch from 7af3013 to 2c6c5b3 Compare February 16, 2024 18:52
@fmeum fmeum changed the title Implement describeKey for UnresolvedSymlinkAction Implement describeKey for more actions Feb 16, 2024
@fmeum fmeum requested a review from tjgq February 16, 2024 18:53
@fmeum
Copy link
Collaborator Author

fmeum commented Feb 16, 2024

I added describeKey implementations for more actions.

@tjgq tjgq 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 Feb 19, 2024
@fmeum
Copy link
Collaborator Author

fmeum commented Feb 19, 2024

@bazel-io fork 7.1.0

@bazel-io bazel-io mentioned this pull request Feb 19, 2024
Closed
@github-actions github-actions bot removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Feb 20, 2024
bazel-io pushed a commit to bazel-io/bazel that referenced this pull request Feb 20, 2024
@fmeum @bazel-io
Implement describeKey for more actions
2b3432f 
Closes bazelbuild#21380.

PiperOrigin-RevId: 608540776
Change-Id: Ic058e199fb68e73018d8ae10cd43d86050ec05d9
@bazel-io bazel-io mentioned this pull request Feb 20, 2024
Merged
@fmeum fmeum deleted the symlink-describe-key branch February 20, 2024 14:29
github-merge-queue bot pushed a commit that referenced this pull request Feb 20, 2024
@bazel-io @fmeum
[7.1.0] Implement describeKey for more actions (#21421)
369d9a1 
Closes #21380.

Commit
adaf299

PiperOrigin-RevId: 608540776
Change-Id: Ic058e199fb68e73018d8ae10cd43d86050ec05d9

Co-authored-by: Fabian Meumertzheim <[email protected]>
@iancha1992
Copy link
Member

The changes in this PR have been included in Bazel 7.1.0 RC1. Please test out the release candidate and report any issues as soon as possible. If you're using Bazelisk, you can point to the latest RC by setting USE_BAZEL_VERSION=last_rc.
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tjgq tjgq tjgq approved these changes

Assignees
No one assigned
Labels
team-Configurability platforms, toolchains, cquery, select(), config transitions
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@fmeum @tjgq @iancha1992