Simplify Storage samples and their tests. #225

jmdobry · 2016-09-30T18:27:57Z

- Using ES2015 (string templates, const, let, and arrow functions
- Unit tests are now really small (they only test the error cases)
- System tests now exercise the CLI of the samples
- Achieved 100% code coverage for files.js, buckets.js, acl.js, and encryption.js
- Samples were updated to follow latest sample usability guidelines

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.

ace-n

LGTM, other than a few minor nits.

ace-n · 2016-10-03T20:37:53Z

storage/acl.js

@@ -11,162 +11,284 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.


Ultra-nit: update LICENSE to use block comments? (That's what #226 does.)

ace-n · 2016-10-03T20:43:26Z

storage/acl.js

@@ -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) => {


IMO this is a fairly long command - what about print-acl instead?

I copied this from the python sample. I figure it was better to be consistent.

ace-n · 2016-10-03T21:19:08Z

storage/encryption.js

+  .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".`)


Typo: encryptiong.

ace-n · 2016-10-03T21:27:06Z

storage/system-test/acl.test.js

+  after((done) => {
+    storage.bucket(bucketName).file(fileName).delete((err) => {
+      assert.ifError(err);
+      setTimeout(() => storage.bucket(bucketName).delete(done), 2000);


I take it you want this to return an error if the bucket delete() operation fails?

codecov-io · 2016-10-03T22:16:48Z

Current coverage is 92.13% (diff: 100%)

No coverage report found for master at db124bc.

Powered by Codecov. Last update db124bc...202eabd

* 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>

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

* 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]

* 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://github.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://github.com/googleapis/nodejs-retail/issues/221)) ([5eb17fc](https://github.com/googleapis/nodejs-retail/commit/5eb17fc9c340cacbb8627442c58597015a2d4134)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

jmdobry assigned ace-n Sep 30, 2016

jmdobry force-pushed the storage branch 2 times, most recently from 8665ba6 to 7b5fa91 Compare September 30, 2016 19:20

ace-n approved these changes Oct 3, 2016

View reviewed changes

jmdobry force-pushed the storage branch from 7b5fa91 to 202eabd Compare October 3, 2016 22:00

Simplify Storage samples and their tests.

202eabd

jmdobry merged commit 3d4bb4d into master Oct 3, 2016

jmdobry deleted the storage branch October 3, 2016 22:41

grayside mentioned this pull request Oct 26, 2022

migrate code from googleapis/nodejs-tasks #2792

Merged

kweinmeister pushed a commit that referenced this pull request Nov 7, 2022

build!: update library to use Node 12 (#225)

ebad8ee

* build!: Update library to use Node 12 Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

kweinmeister mentioned this pull request Nov 7, 2022

[deprecated] migrate code from googleapis/nodejs-media-translation #2803

Closed

ahrarmonsur pushed a commit that referenced this pull request Nov 8, 2022

Release v0.7.1 (#225)

1ea9e50

ahrarmonsur mentioned this pull request Nov 8, 2022

samples: migrate code from googleapis/nodejs-monitoring #2807

Merged

kweinmeister pushed a commit that referenced this pull request Nov 8, 2022

build!: update library to use Node 12 (#225)

e96e2fa

* build!: Update library to use Node 12 Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

kweinmeister mentioned this pull request Nov 8, 2022

migrate code from googleapis/nodejs-media-translation #2808

Merged

kweinmeister pushed a commit that referenced this pull request Nov 8, 2022

build!: update library to use Node 12 (#225)

764866f

* build!: Update library to use Node 12 Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

ace-n pushed a commit that referenced this pull request Nov 10, 2022

Release v0.7.1 (#225)

e25f48b

NimJay pushed a commit that referenced this pull request Nov 10, 2022

refactor: use execSync for tests

9dcebcb

refactor: use execSync for tests #225 automerged by dpebot

NimJay pushed a commit that referenced this pull request Nov 10, 2022

refactor: use execSync for tests

2f0febd

refactor: use execSync for tests #225 automerged by dpebot

ace-n mentioned this pull request Nov 11, 2022

migrate code from googleapis/nodejs-automl #2841

Merged

ace-n pushed a commit that referenced this pull request Nov 14, 2022

fix(deps): update dependency googleapis to v36 (#225)

07d6b98

ace-n mentioned this pull request Nov 14, 2022

migrate code from googleapis/nodejs-compute #2846

Closed

ace-n mentioned this pull request Nov 15, 2022

migrate code from googleapis/nodejs-compute #2853

Merged

ahrarmonsur pushed a commit that referenced this pull request Nov 16, 2022

chore: release 1.5.0 (#225)

d730e67

ahrarmonsur pushed a commit that referenced this pull request Nov 16, 2022

chore: release 1.5.0 (#225)

b0530d0

ahrarmonsur pushed a commit that referenced this pull request Nov 16, 2022

chore: release 1.5.0 (#225)

0300538

ahrarmonsur pushed a commit that referenced this pull request Nov 16, 2022

chore: release 1.5.0 (#225)

608c11e

ahrarmonsur pushed a commit that referenced this pull request Nov 16, 2022

chore: release 1.5.0 (#225)

ba86ad8

ahrarmonsur mentioned this pull request Nov 16, 2022

migrate code from googleapis/nodejs-asset #2856

Merged

grayside pushed a commit that referenced this pull request Nov 16, 2022

chore: release 1.2.0 (#225)

eb11be0

* updated CHANGELOG.md [ci skip] * updated package.json [ci skip] * updated samples/package.json [ci skip]

This was referenced Nov 16, 2022

migrate code from googleapis/nodejs-cloud-container #2859

Closed

migrate code from googleapis/nodejs-cloud-container #2860

Merged

ace-n pushed a commit that referenced this pull request Nov 17, 2022

chore: release 1.2.0 (#225)

9c8bc2d

* updated CHANGELOG.md [ci skip] * updated package.json [ci skip] * updated samples/package.json [ci skip]

grayside pushed a commit that referenced this pull request Nov 17, 2022

chore: release 1.2.0 (#225)

1b6f607

* updated CHANGELOG.md [ci skip] * updated package.json [ci skip] * updated samples/package.json [ci skip]

ace-n pushed a commit that referenced this pull request Nov 17, 2022

chore: release 1.5.0 (#225)

3ae75de

ahrarmonsur pushed a commit that referenced this pull request Nov 17, 2022

chore: release 1.2.0 (#225)

ba6ee7c

* updated CHANGELOG.md [ci skip] * updated package.json [ci skip] * updated samples/package.json [ci skip]

kweinmeister mentioned this pull request Nov 18, 2022

migrate code from googleapis/nodejs-text-to-speech #2872

Merged

kweinmeister pushed a commit that referenced this pull request Nov 18, 2022

chore: release 1.0.0 (#225)

751daa8

NimJay pushed a commit that referenced this pull request Nov 18, 2022

docs(samples): adds samples and tests (#225)

2ec6ef2

NimJay mentioned this pull request Nov 18, 2022

migrate code from googleapis/nodejs-datacatalog #2877

Merged

NimJay pushed a commit that referenced this pull request Nov 18, 2022

docs(samples): adds samples and tests (#225)

5d974d2

ace-n pushed a commit that referenced this pull request Nov 21, 2022

fix(deps): update dependency googleapis to v36 (#225)

55a5655

ace-n pushed a commit that referenced this pull request Nov 21, 2022

fix(deps): update dependency googleapis to v36 (#225)

4237fd7

kweinmeister mentioned this pull request Jan 11, 2023

migrate code from googleapis/nodejs-retail #2975

Merged

Shabirmean pushed a commit that referenced this pull request Feb 16, 2023

fix(deps): update dependency @google-cloud/pubsub to ^0.24.0 (#225)

59e0870

Shabirmean mentioned this pull request Feb 16, 2023

samples: migrate code from googleapis/nodejs-dlp #3035

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simplify Storage samples and their tests. #225

Simplify Storage samples and their tests. #225

jmdobry commented Sep 30, 2016 •

edited

Loading

ace-n left a comment

ace-n Oct 3, 2016

jmdobry Oct 3, 2016

ace-n Oct 3, 2016

jmdobry Oct 3, 2016

ace-n Oct 3, 2016

jmdobry Oct 3, 2016

ace-n Oct 3, 2016

jmdobry Oct 3, 2016

codecov-io commented Oct 3, 2016

		@@ -11,162 +11,284 @@
		// See the License for the specific language governing permissions and
		// limitations under the License.

Simplify Storage samples and their tests. #225

Simplify Storage samples and their tests. #225

Conversation

jmdobry commented Sep 30, 2016 • edited Loading

ace-n left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Oct 3, 2016

Current coverage is 92.13% (diff: 100%)

jmdobry commented Sep 30, 2016 •

edited

Loading