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

Fix/dynamite/header type resolver #1024

Merged
merged 3 commits into from
Oct 27, 2023
Merged

Conversation

Leptopoda
Copy link
Member

This was a funny bug :)

  1. We use generator functions where possible in dynamite for performance reasons. The problem is that they only get invoked lazily (when iterating). This means that they where only executed in the addAll code segment. Because generateImports was called before adding the code generated by generateClients we where not taking the resolved types from these methods into account.

  2. an empty operationId caused our code to fail (fb01f7e)

  3. We didn't hit it but we where calling output.add for the TypeDefs before we added the imports to the output meaning that we could potentially have code before the imports.

@Leptopoda Leptopoda force-pushed the fix/dynamite/header_type_resolver branch from 65efb1a to 59574e7 Compare October 26, 2023 06:11
@Leptopoda
Copy link
Member Author

I hope that we can sort the generated classes in the future to avoid such diffs.
I couldn't really figure out why this happens.

@provokateurin
Copy link
Member

Maybe we should then first figure out how to prevent this diff?

@Leptopoda
Copy link
Member Author

Leptopoda commented Oct 26, 2023

@provokateurin can I get a froce merge?

The generation check for dynamite_e2e timed out and I already fix this in #1014 (which depends on this PR being merged)

EDIT: we might need to find a different solution as the nextcloud tests didn't even run :/

@Leptopoda Leptopoda force-pushed the fix/dynamite/header_type_resolver branch from 59574e7 to 3b6cbd9 Compare October 27, 2023 16:07
@Leptopoda Leptopoda merged commit 514a6dd into main Oct 27, 2023
6 checks passed
@Leptopoda Leptopoda deleted the fix/dynamite/header_type_resolver branch October 27, 2023 17:05
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.

2 participants