chore!: gaxios rebase node 18 #2569
Conversation
fix lint issues
…-storage-fork into gaxios-node-18
product-auto-label
bot
added
size: m
product-auto-label
bot
added
size: xl
| @@ -261,12 +255,19 @@ class Iam { | |||
| qs.optionsRequestedPolicyVersion = options.requestedPolicyVersion; | |||
| } | |||
|
|
|||
| this.request_( | |||
| // eslint-disable-next-line @typescript-eslint/no-floating-promises | |||
There was a problem hiding this comment.
Why is this disable necessary? Looks like errors are being handled in the callback. I'd rather we not add these disables if possible.
There was a problem hiding this comment.
Your concern about the disable is valid. The linter indicates that because this.storageTransport.makeRequest likely returns a Promise, we must handle potential rejections. Even though we have a callback, the Promise itself might reject, and we need to account for that. Therefore, we are suppressing the warning.
There was a problem hiding this comment.
Interesting... this must be a more recent linter change as it didn't complain when I originally wrote storage-transport.ts. Maybe it is worth revisiting the signature / implementation for storage-transport.request. Having to add a disable every time we use a callback to that function is not ideal and has a bit of a code smell to it.
| @@ -347,20 +348,27 @@ class Iam { | |||
| maxRetries = 0; | |||
| } | |||
|
|
|||
| this.request_( | |||
| // eslint-disable-next-line @typescript-eslint/no-floating-promises | |||
There was a problem hiding this comment.
Same comment about this disable.
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> 🦕