Check in Cloud Job Discovery quick start #684

xinyunh0929 · 2018-07-11T23:11:48Z

CJD got Public beta launch so we would like to have our samples checked in as other projects.

xinyunh0929 · 2018-07-12T18:15:38Z

I saw the other team writing the test using 'ava'. Could we use 'mocha' instead?

codecov · 2018-07-12T21:08:54Z

Codecov Report

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

@@           Coverage Diff           @@
##           master     #684   +/-   ##
=======================================
  Coverage   48.52%   48.52%           
=======================================
  Files           1        1           
  Lines          68       68           
=======================================
  Hits           33       33           
  Misses         35       35

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 bfdbab3...b73b47a. Read the comment docs.

ace-n · 2018-07-13T19:39:54Z

IIRC we use Ava since it supports parallel test running - @jmdobry can confirm.

ace-n

Mostly LGTM, but with a few nits.

I'll be OOO next week, so @happyhuman / @fhinkel can take it from here.

ace-n · 2018-07-13T19:40:33Z

jobs/cjd_sample/README.md

+
+# Google Cloud Job Discovery API Samples
+
+Cloud Job Discovery is part of Google for Jobs - a Google-wide commitment to help


Was this autogenerated using repo-tools generate? If not, it should be.

Do you mean the README should be generated from repo-tools generate?
Current README is written by myself. Should I remove it then run repo-tools generate?

generate: Generating lib_readme in: /usr/local/google/home/xinyunh/githubCode/nodeJsPersonalFork/nodejs-docs-samples/jobs/cjd_sample
generate: Compiling: /usr/local/google/home/xinyunh/githubCode/nodeJsPersonalFork/nodejs-docs-samples/jobs/cjd_sample/README.md
generate: Failed to generate: /usr/local/google/home/xinyunh/githubCode/nodeJsPersonalFork/nodejs-docs-samples/jobs/cjd_sample/README.md
generate: Oh no! Generating failed after 0.0230s.
generate: TypeError: Cannot read property 'toUpperCase' of undefined
at Object.exports.createReleaseQualityBadge (/usr/lib/node_modules/@google-cloud/nodejs-repo-tools/cjs/utils/index.js:222:35)
at Object.eval [as main] (eval at createFunctionContext (/usr/lib/node_modules/@google-cloud/nodejs-repo-tools/node_modules/handlebars/dist/cjs/handlebars/compiler/javascript-compiler.js:254:23), :12:88)
at main (/usr/lib/node_modules/@google-cloud/nodejs-repo-tools/node_modules/handlebars/dist/cjs/handlebars/runtime.js:175:32)
at ret (/usr/lib/node_modules/@google-cloud/nodejs-repo-tools/node_modules/handlebars/dist/cjs/handlebars/runtime.js:178:12)
at ret (/usr/lib/node_modules/@google-cloud/nodejs-repo-tools/node_modules/handlebars/dist/cjs/handlebars/compiler/compiler.js:526:21)
at /usr/lib/node_modules/@google-cloud/nodejs-repo-tools/cjs/cli/commands/generate.js:276:46
at Array.forEach ()
at Object.exports.handler (/usr/lib/node_modules/@google-cloud/nodejs-repo-tools/cjs/cli/commands/generate.js:238:16)
at Object.runCommand (/usr/lib/node_modules/@google-cloud/nodejs-repo-tools/node_modules/yargs/lib/command.js:235:44)
at Object.parseArgs [as _parseArgs] (/usr/lib/node_modules/@google-cloud/nodejs-repo-tools/node_modules/yargs/yargs.js:1014:30)

ace-n · 2018-07-13T19:41:21Z

jobs/cjd_sample/quickstart.js

+// [START quickstart]
+
+// Imports the Google APIs client library
+const {google} = require('googleapis');


Do we have a client library specifically for this, as opposed to googleapis? (Paging @theacodes)

ace-n · 2018-07-13T19:42:12Z

jobs/cjd_sample/quickstart.js

+      return;
+    }
+
+    console.log('Request ID: ' + result.data.metadata.requestId);


Two nits:

Use promises

Use template strings

Sorry, I am not too familiar with nodeJs. How could I use promise in this case?

happyhuman · 2018-07-12T18:26:09Z

jobs/cjd_sample/quickstart.js

@@ -0,0 +1,62 @@
+/**
+ * Copyright 2017, Google, Inc.


LLC instead of Inc.

happyhuman · 2018-07-12T18:31:25Z

jobs/cjd_sample/quickstart.js

+    return;
+  }
+
+  if (authClient.createScopedRequired && authClient.createScopedRequired()) {


I am not familiar with authClient object. Does it have a field and a method both named createScopedRequired? Is it needed to check both of them?

Not sure. I just the follow the same way 'kms' did in their samples for authentication.

happyhuman · 2018-07-16T21:52:36Z

jobs/cjd_sample/system-test/quickstart.test.js

@@ -0,0 +1,24 @@
+/**
+ * Copyright 2017, Google, Inc.


happyhuman

LGTM, assuming the comments made by other reviewers are addressed as well.

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>

First CJD sample

5d3df7c

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jul 11, 2018

xinyunh0929 added 2 commits July 12, 2018 11:09

Add README

9f38e5d

fix the lint issue

5e89d3a

Add test

6945b24

ace-n reviewed Jul 13, 2018

View reviewed changes

Modify the sample

6a41037

happyhuman reviewed Jul 16, 2018

View reviewed changes

xinyunh0929 and others added 2 commits July 16, 2018 14:56

Modify the copyright

9131826

Merge branch 'master' into master

2d7a862

happyhuman approved these changes Jul 16, 2018

View reviewed changes

xinyunh0929 added 2 commits July 17, 2018 15:03

Merge branch 'master' into master

af28215

Merge branch 'master' into master

b73b47a

happyhuman merged commit 022055e into GoogleCloudPlatform:master Jul 17, 2018

snyk-bot mentioned this pull request May 26, 2021

[Snyk] Security upgrade got from 9.5.0 to 9.5.1 leejh3224/nodejs-docs-samples#333

Open

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

chore: release 3.3.0 (#684)

1b38029

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>

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

chore: release 3.3.0 (#684)

396d729

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>

ahrarmonsur mentioned this pull request Nov 18, 2022

migrate code from googleapis/nodejs-dialogflow #2873

Merged

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

chore: release 3.3.0 (#684)

af51e46

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>

This was referenced Jul 6, 2023

[Snyk] Security upgrade @google-cloud/logging-winston from 0.11.1 to 5.0.0 pedroarapua/nodejs-docs-samples-gcp#417

Open

[Snyk] Security upgrade @google-cloud/logging-winston from 0.11.1 to 5.0.0 pedroarapua/nodejs-docs-samples-gcp#421

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Check in Cloud Job Discovery quick start #684

Check in Cloud Job Discovery quick start #684

xinyunh0929 commented Jul 11, 2018

xinyunh0929 commented Jul 12, 2018

codecov bot commented Jul 12, 2018 •

edited

Loading

ace-n commented Jul 13, 2018

ace-n left a comment

ace-n Jul 13, 2018

xinyunh0929 Jul 13, 2018

xinyunh0929 Jul 13, 2018

ace-n Jul 13, 2018

ace-n Jul 13, 2018

xinyunh0929 Jul 13, 2018

happyhuman Jul 12, 2018

happyhuman Jul 12, 2018

xinyunh0929 Jul 16, 2018

happyhuman Jul 16, 2018

happyhuman left a comment


		# Google Cloud Job Discovery API Samples

		Cloud Job Discovery is part of Google for Jobs - a Google-wide commitment to help

Check in Cloud Job Discovery quick start #684

Check in Cloud Job Discovery quick start #684

Conversation

xinyunh0929 commented Jul 11, 2018

xinyunh0929 commented Jul 12, 2018

codecov bot commented Jul 12, 2018 • edited Loading

Codecov Report

ace-n commented Jul 13, 2018

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

happyhuman left a comment

Choose a reason for hiding this comment

codecov bot commented Jul 12, 2018 •

edited

Loading