Skip to content
This repository was archived by the owner on Mar 3, 2026. It is now read-only.

fix: add publicUrl to exclude promisifyAll#1339

Merged
stephenplusplus merged 5 commits into
googleapis:masterfrom
fernandolguevara:fernandolguevara-fix-exclude-publicUrl
Dec 1, 2020
Merged

fix: add publicUrl to exclude promisifyAll#1339
stephenplusplus merged 5 commits into
googleapis:masterfrom
fernandolguevara:fernandolguevara-fix-exclude-publicUrl

Conversation

@fernandolguevara

@fernandolguevara fernandolguevara commented Nov 16, 2020

Copy link
Copy Markdown
Contributor

Fixes #1344

I exclude "publicUrl" from the list of functions that will use promises

@fernandolguevara fernandolguevara requested a review from a team November 16, 2020 17:33
@google-cla

google-cla Bot commented Nov 16, 2020

Copy link
Copy Markdown

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@google-cla google-cla Bot added the cla: no This human has *not* signed the Contributor License Agreement. label Nov 16, 2020
@product-auto-label product-auto-label Bot added the api: storage Issues related to the googleapis/nodejs-storage API. label Nov 16, 2020
@AVaksman

Copy link
Copy Markdown
Contributor

@fernandolguevara Thank you for contribution.

Just a couple of nits.

Comment thread src/file.ts Outdated
*/
promisifyAll(File, {
exclude: ['request', 'setEncryptionKey'],
exclude: ['request', 'setEncryptionKey', 'publicUrl'],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please place 'publicUrl' in alphabetic order.

Also please add 'publicUrl' to the list to fix unit tests

assert.deepStrictEqual(options.exclude, ['request', 'setEncryptionKey']);

@google-cla

google-cla Bot commented Nov 23, 2020

Copy link
Copy Markdown

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@google-cla

google-cla Bot commented Nov 23, 2020

Copy link
Copy Markdown

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@codecov

codecov Bot commented Nov 23, 2020

Copy link
Copy Markdown

Codecov Report

Merging #1339 (676a268) into master (91a65f8) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1339   +/-   ##
=======================================
  Coverage   99.00%   99.00%           
=======================================
  Files          14       14           
  Lines       11804    11804           
  Branches      526      595   +69     
=======================================
  Hits        11687    11687           
  Misses        117      117           
Impacted Files Coverage Δ
src/file.ts 99.94% <100.00%> (ø)

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 91a65f8...676a268. Read the comment docs.

Comment thread test/file.ts Outdated
@google-cla

google-cla Bot commented Nov 30, 2020

Copy link
Copy Markdown

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@shaffeeullah

Copy link
Copy Markdown
Contributor

@fernandolguevara friendly bump. If you do not have time to implement the requested changes, let me know and I can take it over from here. However, please sign the CLA so this PR can be merged. Thanks for making these changes!

@fernandolguevara

Copy link
Copy Markdown
Contributor Author

@googlebot I signed it!

@google-cla google-cla Bot 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 1, 2020
Co-authored-by: Sameena Shaffeeullah <shaffeeullah@google.com>
@stephenplusplus stephenplusplus added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 1, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 1, 2020
@stephenplusplus stephenplusplus dismissed AVaksman’s stale review December 1, 2020 16:11

This was addressed

@stephenplusplus stephenplusplus merged commit ea2c2c9 into googleapis:master Dec 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

api: storage Issues related to the googleapis/nodejs-storage API. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

file.publicUrl() returns a Promise instead of the publicUrl

5 participants