feat!: correct wrong return type for listOperations in Operation Client#1729
feat!: correct wrong return type for listOperations in Operation Client#1729
Conversation
|
System tests will fail on this because of the wrong type. This should be expected because gax is pulling in the latest version of system tests for a given API, which will fail because the annotated type is wrong, because this hasn't been generated yet: googleapis/gapic-generator-typescript#1702. The way we'll know this is safe to merge is if Browser tests pass, since I regenerated showcase with the new generator branch, which should be generated with the new future correct types. |
| "@types/node": "^20.11.26", | ||
| "gapic-tools": "./gapic-tools.tgz", | ||
| "typescript": "^5.7.3" | ||
| "@types/mocha": "^10.0.7", |
There was a problem hiding this comment.
This is a lot of dev dependencies changes - should it be?
Also, we should probably be pointing to local gapic-tools
|
|
||
| The only modification is in `package.json`, where it's set to depend on the local | ||
| version of `google-gax` being tested. | ||
| version of `google-gax` being tested. No newline at end of file |
There was a problem hiding this comment.
double check this for a whitespace error - I feel like it shouldn't have anything?
| LocationsClient, | ||
| LocationProtos, | ||
| } from 'google-gax'; | ||
| import type {Callback, CallOptions, Descriptors, ClientOptions, GrpcClientOptions, LROperation, PaginationCallback, GaxCall, IamClient, IamProtos, LocationsClient, LocationProtos} from 'google-gax'; |
There was a problem hiding this comment.
would this change be fixed by a lil npm run fix?
There was a problem hiding this comment.
So this library doesn't have gts, I did run npm fix at the top level but it doesn't touch these files. I think in a future PR maybe I'll add something for linting in the generator, see: https://github.com/googleapis/gapic-generator-typescript/issues/1698
| @@ -1,5 +1,5 @@ | |||
| { | |||
| "extends": "../../node_modules/gts/tsconfig-google.json", | |||
| "extends": "./node_modules/gts/tsconfig-google.json", | |||
There was a problem hiding this comment.
double check the path, not sure if should change
leahecole
left a comment
There was a problem hiding this comment.
LGTM assuming the expected tests pass
Partially fixes: https://github.com/googleapis/gapic-generator-typescript/issues/1693