-
Notifications
You must be signed in to change notification settings - Fork 2k
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: compute_snapshot_schedule_attach/list #3926
base: main
Are you sure you want to change the base?
Conversation
Here is the summary of changes. You are about to add 2 region tags.
This comment is generated by snippet-bot.
|
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.
Hi @gryczj and reviewers,
I'm currently reviewing this pull request and will post my detailed review in a few minutes. In the meantime, here's a quick summary to help everyone get up to speed:
This pull request adds functionality for managing Compute Engine snapshot schedules. Specifically, it introduces three new JavaScript samples:
createSnapshotSchedule.js
: Creates a new snapshot schedule with customizable settings (daily, weekly, or monthly; retention policy; labels).attachSnapshotSchedule.js
: Attaches an existing snapshot schedule to a Compute Engine disk.deleteSnapshotSchedule.js
: Deletes a snapshot schedule. Note that this will fail if the schedule is attached to a disk.
Each sample includes comprehensive comments and error handling. Unit tests are also provided in attachSnapshotSchedule.test.js
to verify the functionality.
The intent is to provide developers with clear, concise examples for working with Compute Engine snapshot schedules using the Node.js client library. This enhances the documentation and makes it easier for users to automate disk snapshots.
And now, a little haiku to lighten the mood:
Code flows like a stream,
Tests run, bugs swim downstream,
Clean code shines so bright.
Looking forward to the discussion!
149ac9e
to
307b652
Compare
@@ -0,0 +1,103 @@ | |||
/* | |||
* Copyright 2024 Google LLC |
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.
sample was added for testing purposes
* You may obtain a copy of the License at | ||
* | ||
* https://www.apache.org/licenses/LICENSE-2.0 | ||
* |
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.
sample was added for testing purposes
84d3b32
to
422a024
Compare
422a024
to
0397137
Compare
Can you please try to fix the conflict @gryczj ? |
@BigBlackWolf PTAL |
Oh, I noticed that wrote a comment, but didn't submit 😅 Thanks, @rsamborski
Joanna is on vacations and I am going to support her with open PRs. @kweinmeister I've resolved the conflicts, do you have any other concerns? |
Gemini encountered an error creating the walkthrough. You can try again by commenting |
@code-review-assist walkthrough |
1 similar comment
@code-review-assist walkthrough |
Gemini encountered an error creating the walkthrough. You can try again by commenting |
@code-review-assist walkthrough |
Gemini encountered an error creating the walkthrough. You can try again by commenting |
@code-review-assist walkthrough |
Gemini encountered an error creating the walkthrough. You can try again by commenting |
The users that are currently failing CLA had CLA signed at the time they were committing the changes. Please force-merge the PR. |
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.