Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add converter for uint8array map to string and serialization for models #1934

Merged

Conversation

qiaozha
Copy link
Member

@qiaozha qiaozha commented Jul 25, 2023

fixes #1933 #1554
This PR:

  1. update the dependencies for core-util
  2. add converter from uint8array map to string and string to uint8array
  3. add serialization for models type in Send function of modular layer.

@qiaozha qiaozha changed the title Add converter for uint8array map to string [Pending core util release]Add converter for uint8array map to string Aug 1, 2023
@@ -25,6 +26,7 @@ export function buildOperationFiles(
needUnexpectedHelper: boolean = true
) {
for (const operationGroup of client.operationGroups) {
utilImports.clear();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it feels odd to have a module-level Set that gets cleared by this method. Does the set need to live outside this method or could it be created when buildOperationFiles is called?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put this outside the scope of this buildOperationFiles functions as explained here https://github.com/Azure/autorest.typescript/pull/1934/files#r1282500160

I need to clear this set because the apis per each operationGroup should be generated in an independent file.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh I see. I'm not wholly against it, but it does create some complexity that could cause unexpected behavior later if someone isn't careful.

Maybe we could later refactor this a bit - I remember when I wrote a prototype TS emitter I had a global bag of context that I passed through every generation method that handled things like tracking deps, etc.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion, will take that into consideration when I try to adopt to TCGC.

@qiaozha qiaozha changed the title [Pending core util release]Add converter for uint8array map to string Add converter for uint8array map to string and serialization for models Aug 4, 2023
@qiaozha qiaozha marked this pull request as ready for review August 4, 2023 07:48
@qiaozha qiaozha merged commit b941319 into Azure:main Aug 8, 2023
28 checks passed
@qiaozha qiaozha deleted the add-converter-for-uint8array-map-to-string branch August 8, 2023 07:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

We are missing a serialize function in the modular layer
4 participants