Functions #245

jmdobry · 2016-10-26T17:13:45Z

No description provided.

googlebot · 2016-10-31T21:12:39Z

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

jmdobry · 2016-11-01T23:44:50Z

@ace-n @jasonpolites PTAL

ace-n · 2016-11-02T21:16:26Z

@jmdobry Missing module is causing CircleCI tests to fail - mind fixing that?

/home/ubuntu/nodejs-docs-samples/bigquery/node_modules/grpc/src/node/extension_binary/grpc_node.node

jmdobry · 2016-11-03T04:13:49Z

Will do

googlebot · 2016-11-05T21:34:38Z

CLAs look good, thanks!

codecov-io · 2016-11-10T20:53:03Z

Current coverage is 92.06% (diff: 98.49%)

Merging #245 into master will decrease coverage by 0.16%

@@             master       #245   diff @@
==========================================
  Files            66         66          
  Lines          2639       2634     -5   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits           2434       2425     -9   
- Misses          205        209     +4   
  Partials          0          0

Powered by Codecov. Last update 2cf8055...8cb8f04

ace-n

(These are comments from looking at the code - I'll run through the tutorials now and post my feedback here once I'm done.)

ace-n · 2016-11-07T20:36:53Z

functions/datastore/test/index.test.js

-}
+describe(`functions:datastore`, () => {
+  it(`set: Set fails without a value`, () => {
+    const expectedMsg = `Value not provided. Make sure you have a "value" property in your request`;


Meta-question/nit: some tests store expected results in their own variables, while others put the constants directly in the assert call. Is this something we should standardize, or am I being pedantic?

I feel like the general pattern I follow is to pull the expected value into a variable when the line is getting long.

ace-n · 2016-11-07T20:42:04Z

functions/errorreporting/index.js

 */
-exports.helloBackgroundError = function helloBackgroundError (context, data) {
+exports.helloBackgroundError = function helloBackgroundError (event, callback) {


Can this be made into an arrow function?

The comment is supposed to be there, indicating that the user would put their own code there.

ace-n · 2016-11-07T20:42:31Z

functions/errorreporting/index.js

-    // All is good, respond with a message
-    return context.success('Hello World!');
+
+    // Do something


Delete this comment, unless there's something missing here?

The comment is supposed to be there, indicating that the user would put their own code there.

ace-n · 2016-11-07T20:52:11Z

functions/gcs/index.js

-    throw new Error('Bucket not provided. Make sure you have a ' +
-      '"bucket" property in your request');
+// [START functions_word_count_stream]
+function getFileStream (file) {


Should we clarify that file is an object or use a different name? (e.g. fileObj)

I'll delete the region tag, as it's not shown in the docs.

ace-n · 2016-11-07T20:56:14Z

functions/gcs/test/index.test.js

-      return fs.createReadStream(filepath, { encoding: 'utf8' });
-    }
+  const file = {
+    createReadStream: () => fs.createReadStream(path.join(__dirname, `../${filename}`), { encoding: `utf8` })


Should we split this into more than 1 line? e.g:

createReadStream: () => fs.createReadStream( path.join(...), { encoding: 'utf8' } )

ace-n · 2016-11-09T00:29:34Z

functions/pubsub/test/index.test.js

+    assert.deepEqual(sample.mocks.res.status.callCount, 1);
+    assert.deepEqual(sample.mocks.res.status.firstCall.args, [500]);
+    assert.deepEqual(sample.mocks.res.send.callCount, 1);
+    assert.deepEqual(sample.mocks.res.send.firstCall.args[0].message, expectedMsg);


Nit: why deepEqual?

ace-n · 2016-11-09T00:29:37Z

functions/pubsub/test/index.test.js

+    assert.deepEqual(sample.mocks.res.status.callCount, 1);
+    assert.deepEqual(sample.mocks.res.status.firstCall.args, [500]);
+    assert.deepEqual(sample.mocks.res.send.callCount, 1);
+    assert.deepEqual(sample.mocks.res.send.firstCall.args[0].message, expectedMsg);


Nit: why deepEqual?

ace-n · 2016-11-09T00:33:01Z

functions/sendgrid/index.js

 */
 function getPayload (requestBody) {
  if (!requestBody.to) {
-    var error = new Error('To email address not provided. Make sure you have a ' +
-      '"to" property in your request');
+    const error = new Error('To email address not provided. Make sure you have a "to" property in your request');


A few nits:

Repetition? (Helper function?)

What's the purpose of using a "const" here, given that you're only using it once?

too much abstraction

I have to first save the error to a variable so I can add the code property to it.

ace-n · 2016-11-11T00:00:50Z

functions/sendgrid/index.js

-      throw new Error('Filename not provided. Make sure you have a ' +
-        '"name" property in your request');
-    }
+exports.sendgridLoad = function sendgridLoad (event) {


These docstrings and parameters don't match up. 😕

ace-n · 2016-11-11T00:03:56Z

functions/slack/index.js


  // Extract the first entity from the result list, if any
-  if (response && response.itemListElement &&
-    response.itemListElement.length) {
+  if (response && response.itemListElement && response.itemListElement.length) {


Maybe clarify the .length part? (e.g. .length > 0?)

ace-n · 2016-11-15T22:30:02Z

functions/background/index.js

 * @returns {Promise}
 */
-exports.helloPromise = function helloPromise (data) {
+exports.helloPromise = function helloPromise (event) {


Do we need the function names here?

Makes for better stack traces.

ace-n · 2016-11-15T22:35:31Z

functions/datastore/README.md


-    * Replace `[YOUR_BUCKET_NAME]` with the name of your Cloud Storage Bucket.
+    * Replace `YOUR_BUCKET_NAME` with the name of your Cloud Storage Bucket.


Does this need to be repeated every time, or can we just have a blanket notice?

jmdobry · 2016-11-16T22:56:50Z

Fixed some stuff related to the OCR sample.

* updated CHANGELOG.md * updated package.json * updated samples/package.json

* chore(main): release 3.0.2 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

🤖 I have created a release \*beep\* \*boop\* --- ### [3.0.1](https://www.github.com/googleapis/nodejs-document-ai/compare/v3.0.0...v3.0.1) (2021-07-21) ### Bug Fixes * **deps:** google-gax v2.17.1 ([#243](https://www.github.com/googleapis/nodejs-document-ai/issues/243)) ([a9e4efd](https://www.github.com/googleapis/nodejs-document-ai/commit/a9e4efd2478def845376a951c40b76e3384a8b29)) * Updating WORKSPACE files to use the newest version of the Typescript generator. ([#245](https://www.github.com/googleapis/nodejs-document-ai/issues/245)) ([06bf319](https://www.github.com/googleapis/nodejs-document-ai/commit/06bf3190cad3c9ce98e13aff718bb11db85dd4e5)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

🤖 I have created a release \*beep\* \*boop\* --- ## [2.3.0](https://www.github.com/googleapis/nodejs-datacatalog/compare/v2.2.0...v2.3.0) (2021-04-27) ### Features * new RenameTagTemplateFieldEnumValue API ([#244](https://www.github.com/googleapis/nodejs-datacatalog/issues/244)) ([1efaf9b](https://www.github.com/googleapis/nodejs-datacatalog/commit/1efaf9be12b84135643761cf01372726ec7dd249)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

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

jmdobry self-assigned this Oct 26, 2016

jmdobry mentioned this pull request Oct 26, 2016

Switch --bucket to --stage-bucket #236

Closed

jmdobry force-pushed the functions branch from 03c06cb to eab1c70 Compare October 28, 2016 20:33

jmdobry assigned ace-n Nov 1, 2016

jmdobry force-pushed the functions branch 2 times, most recently from 6478988 to 02e99ac Compare November 5, 2016 21:34

jmdobry force-pushed the functions branch from 02e99ac to 8cb8f04 Compare November 10, 2016 18:14

ace-n reviewed Nov 11, 2016

View reviewed changes

jmdobry force-pushed the functions branch 2 times, most recently from 1930ef8 to a63abfb Compare November 14, 2016 20:45

ace-n reviewed Nov 15, 2016

View reviewed changes

jmdobry force-pushed the functions branch from a63abfb to 944a486 Compare November 16, 2016 22:46

Refactor Cloud Functions samples.

f7b29b3

jmdobry force-pushed the functions branch from 944a486 to f7b29b3 Compare November 16, 2016 22:50

jmdobry merged commit 3c77013 into master Nov 16, 2016

grayside pushed a commit that referenced this pull request Oct 26, 2022

chore: release 1.1.1 (#245)

2a7e409

* updated CHANGELOG.md * updated package.json * updated samples/package.json

grayside mentioned this pull request Oct 26, 2022

migrate code from googleapis/nodejs-tasks #2792

Merged

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

chore: release 1.1.1 (#245)

1910dab

* updated CHANGELOG.md * updated package.json * updated samples/package.json

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

Refactor Cloud Functions samples. (#245)

9896c7f

ahrarmonsur mentioned this pull request Nov 8, 2022

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

Merged

kweinmeister mentioned this pull request Nov 8, 2022

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

Merged

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

Refactor Cloud Functions samples. (#245)

c2a1f81

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

chore: release 1.6.0 (#245)

8a2b651

NimJay mentioned this pull request Nov 11, 2022

migrate code from googleapis/nodejs-kms #2839

Merged

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

chore: release 1.6.0 (#245)

ed5a4ad

telpirion mentioned this pull request Nov 16, 2022

chore(vision): migrate code from googleapis/nodejs-vision #2858

Merged

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

Refactor Cloud Functions samples. (#245)

49704f5

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 19, 2022

chore: release 1.3.0 (#245)

eb7614f

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

NimJay mentioned this pull request Nov 19, 2022

migrate code from googleapis/nodejs-dataproc #2878

Merged

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

chore: release 1.3.0 (#245)

875aa76

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Functions #245

Functions #245

jmdobry commented Oct 26, 2016

googlebot commented Oct 31, 2016

jmdobry commented Nov 1, 2016

ace-n commented Nov 2, 2016 •

edited

jmdobry commented Nov 3, 2016

googlebot commented Nov 5, 2016

codecov-io commented Nov 10, 2016

ace-n left a comment

ace-n Nov 7, 2016

jmdobry Nov 16, 2016

ace-n Nov 7, 2016

jmdobry Nov 16, 2016

ace-n Nov 7, 2016

jmdobry Nov 16, 2016

ace-n Nov 7, 2016

jmdobry Nov 16, 2016

ace-n Nov 7, 2016

jmdobry Nov 16, 2016

ace-n Nov 9, 2016

jmdobry Nov 16, 2016

ace-n Nov 9, 2016

jmdobry Nov 16, 2016

ace-n Nov 9, 2016

jmdobry Nov 16, 2016

ace-n Nov 11, 2016

jmdobry Nov 16, 2016

ace-n Nov 11, 2016

jmdobry Nov 16, 2016

ace-n Nov 15, 2016

jmdobry Nov 16, 2016

ace-n Nov 15, 2016 •

edited

jmdobry Nov 16, 2016

jmdobry commented Nov 16, 2016


		* Replace `[YOUR_BUCKET_NAME]` with the name of your Cloud Storage Bucket.
		* Replace `YOUR_BUCKET_NAME` with the name of your Cloud Storage Bucket.

Functions #245

Functions #245

Conversation

jmdobry commented Oct 26, 2016

googlebot commented Oct 31, 2016

jmdobry commented Nov 1, 2016

ace-n commented Nov 2, 2016 • edited

jmdobry commented Nov 3, 2016

googlebot commented Nov 5, 2016

codecov-io commented Nov 10, 2016

Current coverage is 92.06% (diff: 98.49%)

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

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

Choose a reason for hiding this comment

ace-n Nov 15, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmdobry commented Nov 16, 2016

ace-n commented Nov 2, 2016 •

edited

ace-n Nov 15, 2016 •

edited