Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: allow calls with no request, add JSON proto #725

Merged
merged 2 commits into from
Aug 2, 2019
Merged

fix: allow calls with no request, add JSON proto #725

merged 2 commits into from
Aug 2, 2019

Conversation

yoshi-automation
Copy link
Contributor

This PR was generated using Autosynth. 🌈

Here's the log from Synthtool:

synthtool > Executing /tmpfs/src/git/autosynth/working_repo/synth.py.
synthtool > Ensuring dependencies.
synthtool > Pulling artman image.
latest: Pulling from googleapis/artman
Digest: sha256:6929f343c400122d85818195b18613330a12a014bffc1e08499550d40571479d
Status: Image is up to date for googleapis/artman:latest
synthtool > Cloning googleapis.
synthtool > Running generator for google/firestore/admin/artman_firestore_v1.yaml.
synthtool > Generated code into /home/kbuilder/.cache/synthtool/googleapis/artman-genfiles/js/firestore-admin-v1.
synthtool > Running generator for google/firestore/artman_firestore.yaml.
synthtool > Generated code into /home/kbuilder/.cache/synthtool/googleapis/artman-genfiles/js/firestore-v1beta1.
synthtool > Running generator for google/firestore/artman_firestore_v1.yaml.
synthtool > Generated code into /home/kbuilder/.cache/synthtool/googleapis/artman-genfiles/js/firestore-v1.
synthtool > Replaced '../../package.json' in dev/src/v1/firestore_admin_client.js.
synthtool > Replaced '../../package.json' in dev/src/v1beta1/firestore_client.js.
synthtool > Replaced '../../package.json' in dev/src/v1/firestore_client.js.
synthtool > Replaced 'return this\\._innerApiCalls\\.listen\\(options\\);' in dev/src/v1beta1/firestore_client.js.
synthtool > Replaced 'return this\\._innerApiCalls\\.listen\\(options\\);' in dev/src/v1/firestore_client.js.
.eslintignore
.eslintrc.yml
.github/ISSUE_TEMPLATE/bug_report.md
.github/ISSUE_TEMPLATE/feature_request.md
.github/ISSUE_TEMPLATE/support_request.md
.jsdoc.js
.kokoro/common.cfg
.kokoro/continuous/node10/common.cfg
.kokoro/continuous/node10/docs.cfg
.kokoro/continuous/node10/lint.cfg
.kokoro/continuous/node10/samples-test.cfg
.kokoro/continuous/node10/system-test.cfg
.kokoro/continuous/node10/test.cfg
.kokoro/continuous/node12/common.cfg
.kokoro/continuous/node12/test.cfg
.kokoro/continuous/node8/common.cfg
.kokoro/continuous/node8/test.cfg
.kokoro/docs.sh
.kokoro/lint.sh
.kokoro/presubmit/node10/common.cfg
.kokoro/presubmit/node10/docs.cfg
.kokoro/presubmit/node10/lint.cfg
.kokoro/presubmit/node10/samples-test.cfg
.kokoro/presubmit/node10/system-test.cfg
.kokoro/presubmit/node10/test.cfg
.kokoro/presubmit/node12/common.cfg
.kokoro/presubmit/node12/test.cfg
.kokoro/presubmit/node8/common.cfg
.kokoro/presubmit/node8/test.cfg
.kokoro/presubmit/windows/common.cfg
.kokoro/presubmit/windows/test.cfg
.kokoro/publish.sh
.kokoro/release/docs.cfg
.kokoro/release/docs.sh
.kokoro/release/publish.cfg
.kokoro/samples-test.sh
.kokoro/system-test.sh
.kokoro/test.bat
.kokoro/test.sh
.kokoro/trampoline.sh
.nycrc
.prettierignore
.prettierrc
CODE_OF_CONDUCT.md
CONTRIBUTING.md
LICENSE
README.md
codecov.yaml
renovate.json
samples/README.md
synthtool > Replaced 'https:\\/\\/cloud\\.google\\.com[\\s\\*]*http:\\/\\/(.*)[\\s\\*]*\\)' in dev/src/v1beta1/doc/google/protobuf/doc_timestamp.js.
synthtool > Replaced 'https:\\/\\/cloud\\.google\\.com[\\s\\*]*http:\\/\\/(.*)[\\s\\*]*\\)' in dev/src/v1/doc/google/protobuf/doc_timestamp.js.
synthtool > No replacements made in **/doc/google/protobuf/doc_timestamp.js for pattern toISOString\], maybe replacement is not longer needed?

> core-js@2.6.9 postinstall /tmpfs/src/git/autosynth/working_repo/node_modules/core-js
> node scripts/postinstall || echo "ignore"

Thank you for using core-js ( https://github.com/zloirock/core-js ) for polyfilling JavaScript standard library!

The project needs your help! Please consider supporting of core-js on Open Collective or Patreon: 
> https://opencollective.com/core-js 
> https://www.patreon.com/zloirock 

Also, the author of core-js ( https://github.com/zloirock ) is looking for a good job -)


> protobufjs@6.8.8 postinstall /tmpfs/src/git/autosynth/working_repo/node_modules/protobufjs
> node scripts/postinstall


> @google-cloud/firestore@2.2.4 prepare /tmpfs/src/git/autosynth/working_repo
> npm run compile


> @google-cloud/firestore@2.2.4 compile /tmpfs/src/git/autosynth/working_repo
> tsc -p . && cp -r dev/protos build && cp -r dev/test/fake-certificate.json build/test/fake-certificate.json && cp dev/src/v1beta1/*.json build/src/v1beta1/ && cp dev/src/v1/*.json build/src/v1/ && cp dev/conformance/test-definition.proto build/conformance && cp dev/conformance/test-suite.binproto build/conformance

npm notice created a lockfile as package-lock.json. You should commit this file.
added 602 packages from 897 contributors and audited 1794 packages in 22.129s
found 0 vulnerabilities


> @google-cloud/firestore@2.2.4 fix /tmpfs/src/git/autosynth/working_repo
> gts fix

synthtool > Cleaned up 2 temporary directories.
synthtool > Wrote metadata to synth.metadata.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 2, 2019
@codecov
Copy link

codecov bot commented Aug 2, 2019

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #725   +/-   ##
=======================================
  Coverage   96.06%   96.06%           
=======================================
  Files          20       20           
  Lines        2234     2234           
  Branches      475      475           
=======================================
  Hits         2146     2146           
  Misses         22       22           
  Partials       66       66

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 169286d...6db8c22. Read the comment docs.

2 similar comments
@codecov
Copy link

codecov bot commented Aug 2, 2019 •
edited
Loading

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #725   +/-   ##
=======================================
  Coverage   96.06%   96.06%           
=======================================
  Files          20       20           
  Lines        2234     2234           
  Branches      475      475           
=======================================
  Hits         2146     2146           
  Misses         22       22           
  Partials       66       66

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 169286d...6db8c22. Read the comment docs.

@codecov
Copy link

codecov bot commented Aug 2, 2019

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #725   +/-   ##
=======================================
  Coverage   96.06%   96.06%           
=======================================
  Files          20       20           
  Lines        2234     2234           
  Branches      475      475           
=======================================
  Hits         2146     2146           
  Misses         22       22           
  Partials       66       66

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 169286d...6db8c22. Read the comment docs.

@codecov
Copy link

codecov bot commented Aug 2, 2019 •
edited
Loading

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #725   +/-   ##
=======================================
  Coverage   96.06%   96.06%           
=======================================
  Files          20       20           
  Lines        2234     2234           
  Branches      475      475           
=======================================
  Hits         2146     2146           
  Misses         22       22           
  Partials       66       66

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 5b7e6e0...ade5107. Read the comment docs.

@codecov
Copy link

codecov bot commented Aug 2, 2019

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #725   +/-   ##
=======================================
  Coverage   96.06%   96.06%           
=======================================
  Files          20       20           
  Lines        2234     2234           
  Branches      475      475           
=======================================
  Hits         2146     2146           
  Misses         22       22           
  Partials       66       66

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 169286d...6db8c22. Read the comment docs.

@alexander-fenster alexander-fenster changed the title [CHANGE ME] Re-generated to pick up changes in the API or client library generator. fix: allow calls with no request, add JSON proto Aug 2, 2019
@JustinBeckwith JustinBeckwith merged commit 8b0624b into master Aug 2, 2019
@jakeleventhal
Copy link

Can someone please explain what is meant by "Allow calls with no request" on a functional level?

@alexander-fenster
Copy link
Contributor

@jakeleventhal This might not make any sense for this particular library. Some API calls in other libraries allow empty request objects (e.g. listVoices in Google Cloud Text-To-Speech here https://github.com/googleapis/nodejs-text-to-speech/blob/3d37c3cfc53a420b722160397ddef97fc25af298/src/v1/text_to_speech_client.js#L218) and the generated client code required users to pass {} as a parameter. This small fix to the code generator adds

request = request || {};

so that it's possible to call client.listVoices() instead of client.listVoices({}).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexander-fenster alexander-fenster alexander-fenster approved these changes

Assignees
No one assigned
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@yoshi-automation @jakeleventhal @alexander-fenster @JustinBeckwith @googlebot