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

Uh oh!

Check in Cloud Job Discovery quick start #684

Check in Cloud Job Discovery quick start #684

Uh oh!

xinyunh0929 commented Jul 11, 2018

Uh oh!

xinyunh0929 commented Jul 12, 2018

Uh oh!

codecov bot commented Jul 12, 2018 •

edited

Loading

Uh oh!

ace-n commented Jul 13, 2018

Uh oh!

ace-n left a comment

Uh oh!

ace-n Jul 13, 2018

Uh oh!

xinyunh0929 Jul 13, 2018

Uh oh!

xinyunh0929 Jul 13, 2018

Uh oh!

ace-n Jul 13, 2018

Uh oh!

ace-n Jul 13, 2018

Uh oh!

xinyunh0929 Jul 13, 2018

Uh oh!

happyhuman Jul 12, 2018

Uh oh!

happyhuman Jul 12, 2018

Uh oh!

xinyunh0929 Jul 16, 2018

Uh oh!

happyhuman Jul 16, 2018

Uh oh!

happyhuman left a comment

Uh oh!

Uh oh!


		# 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

Uh oh!

Conversation

xinyunh0929 commented Jul 11, 2018

Uh oh!

xinyunh0929 commented Jul 12, 2018

Uh oh!

codecov bot commented Jul 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ace-n commented Jul 13, 2018

Uh oh!

ace-n left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

happyhuman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Jul 12, 2018 •

edited

Loading