-
Notifications
You must be signed in to change notification settings - Fork 2k
feat: add compute_reservations_delete/get/list #3834
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
Conversation
|
Here is the summary of changes. You are about to add 3 region tags.
This comment is generated by snippet-bot.
|
776acce to
a5db741
Compare
| throw new Error('Reservation was not deleted.'); | ||
| } catch (error) { | ||
| // Assert that the error message indicates the reservation wasn't found | ||
| expect(error.message).to.include( |
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.
Great PR; the only real nit I have is using chai just to make an expect call. This can easily be handled without an extra package dependency (I realize the package dependency was already there for other tests, but String.prototype.includes() would do what you need here). Ideally, we'd have fewer test dependencies and rely more on core APIs, a goal I think we'll articulate more. That being said, maybe keep it in mind for future refactoring and let's get your queue merged! :)
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.