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

[credentialhelper] Update flag doc to point to more convenient usage instructions #21382

Closed
wants to merge 2 commits into from

Conversation

Yannic
Copy link
Contributor

@Yannic Yannic commented Feb 16, 2024

Arguably, we should have a dedicated page for this on bazel.build, but until we have that, this post seems like a better source for users than the original design doc.

…instructions

Arguably, we should have a dedicated page for this on `bazel.build`, but until we have that, this post seems like a better source for users than the original design doc.
@github-actions github-actions bot added the awaiting-review PR is awaiting review from an assigned reviewer label Feb 16, 2024
@tjgq
Copy link
Contributor

tjgq commented Feb 16, 2024

@bazel-io fork 7.1.0

@tjgq tjgq added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally team-Remote-Exec Issues and PRs for the Execution (Remote) team and removed awaiting-review PR is awaiting review from an assigned reviewer labels Feb 16, 2024
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.

While the blog post clarifies how to use a credential helper, it doesn't describe how to write one. But I agree that the proposal isn't the right artifact to link to. How about we also link to https://github.com/EngFlow/credential-helper-spec/blob/main/spec.md?

@tjgq tjgq added awaiting-user-response Awaiting a response from the author and removed awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally labels Feb 16, 2024
@Yannic
Copy link
Contributor Author

Yannic commented Feb 19, 2024

While the blog post clarifies how to use a credential helper, it doesn't describe how to write one. But I agree that the proposal isn't the right artifact to link to. How about we also link to https://github.com/EngFlow/credential-helper-spec/blob/main/spec.md?

SG, done. PTAL

@tjgq tjgq added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-user-response Awaiting a response from the author labels Feb 20, 2024
@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
…instructions

Arguably, we should have a dedicated page for this on `bazel.build`, but until we have that, this post seems like a better source for users than the original design doc.

Closes bazelbuild#21382.

PiperOrigin-RevId: 608747673
Change-Id: Iaca0dc8364a6d622f685bdd53eea54068c4ea94c
Wyverald pushed a commit that referenced this pull request Feb 20, 2024
…t usage instructions (#21441)

Arguably, we should have a dedicated page for this on `bazel.build`, but
until we have that, this post seems like a better source for users than
the original design doc.

Closes #21382.

Commit
f1b02a2

PiperOrigin-RevId: 608747673
Change-Id: Iaca0dc8364a6d622f685bdd53eea54068c4ea94c

Co-authored-by: Yannic <yannic@engflow.com>
@Yannic Yannic deleted the patch-9 branch February 20, 2024 23:06
@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
Labels
team-Remote-Exec Issues and PRs for the Execution (Remote) team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants