-
Notifications
You must be signed in to change notification settings - Fork 130
Modify NodeJS codegen to fit with gcloud-node design. #392
Conversation
The generated result is reviewed at googleapis/google-cloud-node#1476 and probably some details will change later. |
- retire use_pbjs flag -- it's not used anymore, and it does not fit with the new pattern of Gapic code. - update the grpc package template: this fits with the proto packages used by gcloud-node. - update gax package template: this will work well with the new pattern of Gapic code, as you can see in googleapis/gapic-generator#392 or googleapis/google-cloud-node#1476
- The new style exports a function instead of the class. The exported function will take the auth context as a parameter and returns builder function. - Files will be under a sub directory with versioned name, so that it can be exposed as a sub package of the API package. - A few misc changes and style changes made to pass lint checker in gcloud.
- retire use_pbjs flag -- it's not used anymore, and it does not fit with the new pattern of Gapic code. - update the grpc package template: this fits with the proto packages used by gcloud-node. - update gax package template: this will work well with the new pattern of Gapic code, as you can see in googleapis/gapic-generator#392 or googleapis/google-cloud-node#1476
Current coverage is 81.40% (diff: 83.33%)@@ master #392 diff @@
==========================================
Files 183 185 +2
Lines 6904 6943 +39
Methods 0 0
Messages 0 0
Branches 900 905 +5
==========================================
+ Hits 5603 5652 +49
+ Misses 1026 1017 -9
+ Partials 275 274 -1
|
* The version of the calling service. | ||
*/ | ||
function {@serviceName}(opts) { | ||
function {@serviceName}(gaxGrpc, grpcClient, opts) { |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
- constructor parameters are not actually useful now. They are parameters to the builder function actually, so I moved there. - build() is not a function, now it's a class. This way, JSDoc can generate documentations for its methods (like builder function). - add an example clause in the constructor doc comments, so that documentation says the details of how to create an instance of the API client.
Modified the code a bit. Please take another look. Here are the key points: var pubsubV1 = require('@google-cloud/pubsub').v1();
var publisher = pubsubV1.publisherApi();
publisher.createTopic(...); So now,
|
if (names.size() >= 3) { | ||
return lowerUnderscoreToLowerCamel( | ||
names.get(names.size() - 3) + "_" + names.get(names.size() - 2)); | ||
} else if (names.size() >= 2) { |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Two nits, otherwise LGTM |
LGTM if build passes |
- retire use_pbjs flag -- it's not used anymore, and it does not fit with the new pattern of Gapic code. - update the grpc package template: this fits with the proto packages used by gcloud-node. - update gax package template: this will work well with the new pattern of Gapic code, as you can see in googleapis/gapic-generator#392 or googleapis/google-cloud-node#1476
- retire use_pbjs flag -- it's not used anymore, and it does not fit with the new pattern of Gapic code. - update the grpc package template: this fits with the proto packages used by gcloud-node. - update gax package template: this will work well with the new pattern of Gapic code, as you can see in googleapis/gapic-generator#392 or googleapis/google-cloud-node#1476 - add 'env' parameter to dependency_out protoc
exported function will take the auth context as a parameter
and returns builder function.
so that it can be exposed as a sub package of the API package.
checker in gcloud.