feat: allow options to ServiceObject methods#349
feat: allow options to ServiceObject methods#349JustinBeckwith merged 10 commits intogoogleapis:masterfrom
Conversation
src/service-object.ts
Outdated
| } | ||
|
|
||
| // tslint:disable-next-line:no-any | ||
| export type GetMetadataOptions = any; |
There was a problem hiding this comment.
I think maybe it would be better to use object than any here. I think it is safe to assume that we'll only ever allow objects to be passed in (as opposed to strings, numbers, etc.).
src/service-object.ts
Outdated
| // tslint:disable-next-line:no-any | ||
| export type Metadata = any; | ||
| // tslint:disable-next-line:no-any | ||
| export type SetMetadataOptions = any; |
There was a problem hiding this comment.
See comments for GetMetadataOptions.
src/service-object.ts
Outdated
| optionsOrCallback?: GetMetadataOptions|MetadataCallback, | ||
| callback?: MetadataCallback): Promise<MetadataResponse>|void { | ||
| let options: GetMetadataOptions = {}; | ||
| if (typeof optionsOrCallback === 'function') { |
There was a problem hiding this comment.
Consider using maybeOptionsOrCallback to simplify this logic!
src/service-object.ts
Outdated
| metadata: Metadata, | ||
| optionsOrCallback?: SetMetadataOptions|MetadataCallback, | ||
| callback?: MetadataCallback): Promise<SetMetadataResponse>|void { | ||
| const options = |
There was a problem hiding this comment.
Consider using maybeOptionsOrCallback to simplify this logic!
src/service-object.ts
Outdated
| request(opts: r.Options): DecorateRequestOptions; | ||
| } | ||
|
|
||
| // tslint:disable-next-line:no-any |
There was a problem hiding this comment.
I think you can get rid of this now?
JustinBeckwith
left a comment
There was a problem hiding this comment.
This LGTM, as long as we're not causing too much pain breaking our upstream dependencies :)
| */ | ||
| protected poll_(callback: MetadataCallback): void { | ||
| this.getMetadata((err, body) => { | ||
| this.getMetadata((err: ApiError, body: Metadata) => { |
There was a problem hiding this comment.
huh, the compiler should be able to infer the types for the callback. Weird that you had to add this.
|
@JustinBeckwith @callmehiphop -- tests added. PTAfinalL. It would be great to get this in before the next release (#350). |
To Dos
This allows
optionsto be passed to several methods on ServiceObject:For Storage, in order to support the
userProjectobject, we ended up duplicating many of these methods, so that we could pass extra parameters along with the request as query string parameters. With this change, we will just support that here, where it probably belongs.Sending this in early for feedback before I carry on with the other methods and tests.