-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
feat: Added samples for secrets with labels #3728
Conversation
Here is the summary of changes. You are about to add 4 region tags.
This comment is generated by snippet-bot.
|
6bdfc54
to
6af0af3
Compare
|
||
'use strict'; | ||
|
||
async function main( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit (for all samples): I don't think it's good practice to have default values since the sample will fail confusingly for users not running it with the correct values. I think its better to leave them empty so that the user gets a message as to why it fails. Plus, you have the sample values in the comments anyways.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
Hi @vipul7499, the CI test is failing because certain workflows can't be run from a fork. Please resubmit this pull request using a branch on this repo instead of your fork. I confirmed that your pull request will pass tests once you do this. Thanks! |
@vipul7499 LGTM from dee-infra. Will look out for your new PR from the branch as @subfuzion mentioned. |
Hey, @Sita04 @subfuzion I am unable to create a branch because i do not have required permissions. |
Please work with your team to follow the process for getting access already mentioned in internal chat. Thank you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vipul7499 we need to chat about this set of samples.
Hey @iennae , Can you please post your concern regarding the samples? |
22c26c4
into
GoogleCloudPlatform:SM-labels
@subfuzion |
* feat: Added samples for secrets with labels (#3728) * feat: Added samples for secrets with labels * feat: Added samples for secrets with labels * fix: Linting fix * fix: remoing unwanted change * fix: remove default values for params --------- Co-authored-by: Adam Ross <adamross@google.com> * fix: updated tests --------- Co-authored-by: Adam Ross <adamross@google.com> Co-authored-by: Tony Pujals <subfuzion@users.noreply.github.com>
Description
Fixes #
Note: Before submitting a pull request, please open an issue for discussion if you are not associated with Google.
Checklist
npm test
(see Testing)npm run lint
(see Style)GoogleCloudPlatform/nodejs-docs-samples
. Not a fork.