[ContainerRegistry] switch to use official TS code generator #15777
Conversation
and re-generate code.
| ? `${options.userAgentOptions.userAgentPrefix} ${packageDetails}` | ||
| : `${packageDetails}`; | ||
| options.userAgentOptions = { | ||
| userAgentPrefix: userAgentPrefix |
There was a problem hiding this comment.
Were we setting the user agent string in the convenience layer before? If so, are we now ending up with duplicates?
There was a problem hiding this comment.
You are right. This would lead to duplicate of azsdk-js-container-registry/1.0.0-beta.4. In convenience layer we already append library info to options.userAgentOptions.userAgentPrefix.
@sarangan12 what are your thoughts? Update code gen to not append packageDetails when userAgentPrefix is passed via options, or update convenice layer to not append library info?
There was a problem hiding this comment.
We were thinking of convenience layer not appending library info. From the very beginning this was one of the things that we felt could be generated. Now that we are generating, we should make use of that :)
The only thing that should be added as a prefix to this is what the user provides
There was a problem hiding this comment.
Makes sense. I will update the convenience layer.
There was a problem hiding this comment.
Found a bug in the codegen: my code is creating two generated clients using the same options. However, as the generated code mutates my option object, I end up having duplicate library info. I fixed it by making an updated copy of the options bag.
- options.userAgentOptions = {
- userAgentPrefix: userAgentPrefix
- };
-
const optionsWithDefaults = {
...defaults,
...options,
+ userAgentOptions: {
+ userAgentPrefix
+ },
baseUri: options.endpoint || "{url}"
};There was a problem hiding this comment.
|
Did we use the same swagger file as before or were there changes to the swagger file too? Am wondering if it would be easier to review the PR if we used the same swagger file with the new code generator. Then we can attribute all the changes to the code generator and not the swagger changes |
Yes same swagger. The only manual change I made is switching from using my local autorest.typescript to the released package. |
sdk/containerregistry/container-registry/src/generated/generatedClientContext.ts
Show resolved
Hide resolved
|
Lets go! |
| { | ||
| "path": "src/constants.ts", | ||
| "prefix": "SDK_VERSION" | ||
| "prefix": "const packageDetails" |
There was a problem hiding this comment.
@jeremymeng do you remember why having the const part here?
There was a problem hiding this comment.
@deyaaeldeen
Because this metadata is named "prefix" and the line
const packageDetails =azsdk-js-container-registry/1.0.0-beta.4;
contains const packageDetails at the beginning. However it doesn't have to be here, if packageDetails alone also works when the bot is bumping versions.
and re-generate code.
The only manual change is in swagger\README.md. Rest are from generated code.