-
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
Simplify Storage samples and their tests. #225
Conversation
8665ba6
to
7b5fa91
Compare
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.
LGTM, other than a few minor nits.
@@ -11,162 +11,284 @@ | |||
// See the License for the specific language governing permissions and | |||
// limitations under the License. |
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.
Ultra-nit: update LICENSE to use block comments? (That's what #226 does.)
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.
Done
@@ -175,45 +297,49 @@ var program = module.exports = { | |||
|
|||
cli | |||
.demand(1) | |||
.command('add <entity> <role>', 'Add access controls on a bucket or file.', {}, function (options) { | |||
program.addAccessControl(utils.pick(options, ['entity', 'role', 'bucket', 'default', 'file']), utils.makeHandler()); | |||
.command(`print-bucket-acl <bucketName>`, `Prints the ACL for a bucket.`, {}, (opts) => { |
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.
IMO this is a fairly long command - what about print-acl
instead?
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.
I copied this from the python sample. I figure it was better to be consistent.
.example(`node $0 generate-encryption-key`, `Generate a sample encryption key.`) | ||
.example(`node $0 upload my-bucket ./resources/test.txt file_encrypted.txt QxhqaZEqBGVTW55HhQw9Q=`, `Encrypts and uploads "resources/test.txt" to "gs://my-bucket/file_encrypted.txt".`) | ||
.example(`node $0 download my-bucket file_encrypted.txt ./file.txt QxhqaZEqBGVTW55HhQw9Q=`, `Decrypts and downloads "gs://my-bucket/file_encrypted.txt" to "./file.txt".`) | ||
.example(`node $0 rotate my-bucket file_encrypted.txt QxhqaZEqBGVTW55HhQw9Q= SxafpsdfSDFS89sds9Q=`, `Rotates encryptiong keys for "gs://my-bucket/file_encrypted.txt".`) |
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.
Typo: encryptiong.
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
after((done) => { | ||
storage.bucket(bucketName).file(fileName).delete((err) => { | ||
assert.ifError(err); | ||
setTimeout(() => storage.bucket(bucketName).delete(done), 2000); |
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.
I take it you want this to return an error if the bucket delete()
operation fails?
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
Current coverage is 92.13% (diff: 100%)
|
* docs(samples): Update ReadMe and comments for samples * docs(samples): Update ReadMe and comments for samples * Update bodyparser and comment * Fix merge error * Add comment for bodyParser * Update comment
* docs(samples): Update ReadMe and comments for samples * docs(samples): Update ReadMe and comments for samples * Update bodyparser and comment * Fix merge error * Add comment for bodyParser * Update comment
* build!: Update library to use Node 12 Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
* build!: Update library to use Node 12 Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
* build!: Update library to use Node 12 Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
refactor: use execSync for tests #225 automerged by dpebot
refactor: use execSync for tests #225 automerged by dpebot
* wrap await with async * wrap await * lint * license * init without assigning null * init without assigning null * change outside function to sync * comply to style guide & lint * copyright header * copyright header * revert to 2018
* wrap await with async * wrap await * lint * license * init without assigning null * init without assigning null * change outside function to sync * comply to style guide & lint * copyright header * copyright header * revert to 2018
* wrap await with async * wrap await * lint * license * init without assigning null * init without assigning null * change outside function to sync * comply to style guide & lint * copyright header * copyright header * revert to 2018
* wrap await with async * wrap await * lint * license * init without assigning null * init without assigning null * change outside function to sync * comply to style guide & lint * copyright header * copyright header * revert to 2018
* wrap await with async * wrap await * lint * license * init without assigning null * init without assigning null * change outside function to sync * comply to style guide & lint * copyright header * copyright header * revert to 2018
* updated CHANGELOG.md [ci skip] * updated package.json [ci skip] * updated samples/package.json [ci skip]
* wrap await with async * wrap await * lint * license * init without assigning null * init without assigning null * change outside function to sync * comply to style guide & lint * copyright header * copyright header * revert to 2018
* wrap await with async * wrap await * lint * license * init without assigning null * init without assigning null * change outside function to sync * comply to style guide & lint * copyright header * copyright header * revert to 2018
* updated CHANGELOG.md [ci skip] * updated package.json [ci skip] * updated samples/package.json [ci skip]
* updated CHANGELOG.md [ci skip] * updated package.json [ci skip] * updated samples/package.json [ci skip]
* wrap await with async * wrap await * lint * license * init without assigning null * init without assigning null * change outside function to sync * comply to style guide & lint * copyright header * copyright header * revert to 2018
* updated CHANGELOG.md [ci skip] * updated package.json [ci skip] * updated samples/package.json [ci skip]
🤖 I have created a release *beep* *boop* --- ## [2.1.1](https://togithub.com/googleapis/nodejs-retail/compare/v2.1.0...v2.1.1) (2022-11-16) ### Bug Fixes * **deps:** Use google-gax v3.5.2 ([#221](https://togithub.com/googleapis/nodejs-retail/issues/221)) ([5eb17fc](https://togithub.com/googleapis/nodejs-retail/commit/5eb17fc9c340cacbb8627442c58597015a2d4134)) --- This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).
const
,let
, and arrow functionsfiles.js
,buckets.js
,acl.js
, andencryption.js
I left the Storage Transfer API samples alone. I don't want to mess with them until the google-cloud-node has support for the Storage Transfer API.