feat(samples): adds read and filter snippets by billyjacobson · Pull Request #586 · googleapis/nodejs-bigtable

billyjacobson · 2019-11-26T17:15:46Z

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
Ensure the tests and linter pass
Code coverage does not decrease (if any source code was changed)
Appropriate docs were updated (if necessary)

Fixes #<issue_number_goes_here> 🦕

…ds snippets

codecov · 2019-11-26T17:25:41Z

Codecov Report

Merging #586 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #586   +/-   ##
=======================================
  Coverage   87.24%   87.24%           
=======================================
  Files          33       33           
  Lines        2046     2046           
  Branches      373      373           
=======================================
  Hits         1785     1785           
  Misses        233      233           
  Partials       28       28

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5c22968...6b6d7d6. Read the comment docs.

samples/filterSnippets.js

samples/test/filters.js

samples/test/reads.js

samples/writeBatch.js

* feat: add table level IAM policy controls * lint: lint * feat: add instance level IAM policy * docs: update description * docs: add document snippets * lint: lint

Fixed headers Removed unnceccessary asyncs Arrow functions where possible

googlebot · 2019-11-27T01:54:21Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

samples/document-snippets/instance.js

samples/filterSnippets.js

billyjacobson · 2019-11-27T02:11:00Z

I'm unable to pass lint because of how I am calling the functions

bcoe

thoughts inline 👍

samples/test/filters.js

googlebot · 2019-12-03T00:37:53Z

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

samples/__snapshots__/reads.js

bcoe

This is looking good, thanks for working with me and trying out the snapshots.

My only concern, from a stylistic point of view, is that we have been moving towards a single sample per-file (which would have addressed my eval comments). However, I understand the approach you're taking, given that there are many many small methods you're writing samples for ...

tldr; if we move to a switch statement, I'm 👍 on multiple samples in a file.

samples/readSnippets.js

googlebot · 2019-12-03T20:11:10Z

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

googlebot · 2019-12-03T20:50:37Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

googlebot · 2019-12-03T21:12:17Z

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

samples/__snapshots__/filters.js

JustinBeckwith · 2019-12-03T21:26:40Z

samples/readSnippets.js

+    // [START bigtable_reads_row]
+    case 'readRow': {
+      const rowkey = 'phone#4c410523#20190501';
+


Can we delete the unnecessary newlines here?

I like to separate the variable declarations from the API call, but I deleted the line between the call and print so it would be consistent across the samples

samples/readSnippets.js

samples/package.json

googlebot · 2019-12-03T22:15:37Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

googlebot · 2019-12-03T22:23:21Z

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

billyjacobson added 5 commits November 19, 2019 16:39

Reads and filters WIP, working Reads tests and current format for Rea…

29d42e9

…ds snippets

working tests for solo filters

7bec364

Composing filters

ac2c18d

add bulk region tags

989e51f

Change testing instance

88f4247

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Nov 26, 2019

Linted

9d6a0a4

Merge branch 'master' into reads

c1b1b13

billyjacobson requested a review from JustinBeckwith November 26, 2019 17:25

billyjacobson marked this pull request as ready for review November 26, 2019 17:26

JustinBeckwith suggested changes Nov 26, 2019

View reviewed changes

AVaksman and others added 3 commits November 26, 2019 20:53

feat: implement instance and table level IAM policy (#584)

0bb0362

* feat: add table level IAM policy controls * lint: lint * feat: add instance level IAM policy * docs: update description * docs: add document snippets * lint: lint

Linted

71d688a

Fixed headers Removed unnceccessary asyncs Arrow functions where possible

Arrow functions

397fcb6

googlebot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Nov 27, 2019

JustinBeckwith suggested changes Nov 27, 2019

View reviewed changes

samples/document-snippets/instance.js Show resolved Hide resolved

samples/document-snippets/instance.js Show resolved Hide resolved

samples/filterSnippets.js Outdated Show resolved Hide resolved

Merge branch 'master' into reads

abf66df

destructure Bigtable require

7c7afc0

bcoe suggested changes Nov 27, 2019

View reviewed changes

samples/test/filters.js Outdated Show resolved Hide resolved

Using snapshots for testing

c990522

bcoe added the cla: yes This human has signed the Contributor License Agreement. label Dec 3, 2019

googlebot removed the cla: no This human has *not* signed the Contributor License Agreement. label Dec 3, 2019

bcoe reviewed Dec 3, 2019

View reviewed changes

samples/__snapshots__/reads.js Show resolved Hide resolved

bcoe suggested changes Dec 3, 2019

View reviewed changes

samples/readSnippets.js Outdated Show resolved Hide resolved

bcoe added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 3, 2019

googlebot added the cla: no This human has *not* signed the Contributor License Agreement. label Dec 3, 2019

bcoe added cla: yes This human has signed the Contributor License Agreement. kokoro:force-run Add this label to force Kokoro to re-run the tests. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Dec 3, 2019

kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 3, 2019

bcoe changed the title ~~Read and Filter snippets~~ feat(samples): adds read and filter snippets Dec 3, 2019

Fix timezone issue for tests

a333e1c

googlebot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Dec 3, 2019

Lint snapshot files

2e08a28

bcoe added cla: yes This human has signed the Contributor License Agreement. kokoro:force-run Add this label to force Kokoro to re-run the tests. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Dec 3, 2019

kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 3, 2019

bcoe approved these changes Dec 3, 2019

View reviewed changes

JustinBeckwith approved these changes Dec 3, 2019

View reviewed changes

bcoe reviewed Dec 3, 2019

View reviewed changes

samples/package.json Outdated Show resolved Hide resolved

Clean up loops, add update snapshot method

6b6d7d6

googlebot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Dec 3, 2019

bcoe added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Dec 3, 2019

billyjacobson merged commit 896d024 into master Dec 3, 2019

bcoe added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 3, 2019

stephenplusplus deleted the reads branch August 10, 2020 17:37

Conversation

billyjacobson commented Nov 26, 2019

Uh oh!

codecov bot commented Nov 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

googlebot commented Nov 27, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

billyjacobson commented Nov 27, 2019

Uh oh!

bcoe left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

googlebot commented Dec 3, 2019

Uh oh!

Uh oh!

bcoe left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

googlebot commented Dec 3, 2019

Uh oh!

googlebot commented Dec 3, 2019

Uh oh!

googlebot commented Dec 3, 2019

Uh oh!

Uh oh!

JustinBeckwith Dec 3, 2019

Choose a reason for hiding this comment

Uh oh!

billyjacobson Dec 3, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

googlebot commented Dec 3, 2019

Uh oh!

googlebot commented Dec 3, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

codecov bot commented Nov 26, 2019 •

edited

Loading