googleapis / packman

Google API package creator
https://npmjs.com/package/googleapis-packman
Apache License 2.0
13 stars 18 forks source link

Nodejs: Don't use lodash.union if it is unnecessary. #144

Closed landrito closed 7 years ago

landrito commented 8 years ago

What

When generating the index.js for an api, we are using lodash.union to combine the sets of scopes. This is unneeded for situations where there is no scopes or there is only one service which a non empty set of scopes.

Example

This can be seen in monitoring where this line is uneeded since the scopes exported by group-service-api and metric-service-api are empty.

Expected

Packman should be smart enough to see that there is no scopes exported, to simply set ALL_SCOPES to [], and to not require lodash.union in index.js or as a dependency.

landrito commented 8 years ago

cc @jmuk

jmuk commented 8 years ago

Oh, I see. Also some APIs have exactly same set of scopes among services (like pubsub: https://github.com/GoogleCloudPlatform/google-cloud-node/blob/master/packages/pubsub/src/v1/publisher_api.js#L56 and https://github.com/GoogleCloudPlatform/google-cloud-node/blob/master/packages/pubsub/src/v1/subscriber_api.js#L52). In that case actually union isn't necessary.

However, it's hard to do that because packman does not receive gapic config. I think the way is to move the generation of the index.js to toolkit (codegen) and add behaviors like that.

landrito commented 8 years ago

Agreed. I am filing this issue here until it is moved into toolkit. This should be resolved by the implementation of this.

landrito commented 7 years ago

This seems to be fixed with https://github.com/googleapis/toolkit/pull/867.