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

TNT-40793, TNT-40657 - Regenerate openapi models #43

Merged
merged 1 commit into from
May 13, 2021

Conversation

dcottingham
Copy link
Contributor

@dcottingham dcottingham commented May 6, 2021

Regenerated openapi models to include several recent updates. It looks like there are potentially breaking changes in here due to types in some models being updated. I can remove them if we think they are unnecessary, or we could do a major version bump next release.

Side Note: The openapi-generator-cli that we use for the other SDKs is pinned to an older version of the typescript-fetch generator, which seems to be a problem. The pinned version results in slightly incorrect models, so I used a workaround. We can discuss and if agreed upon I will update the README. At this point in time we won't be able keep the openapi client generation in-sync with the other SDKs because of this.

@dcottingham dcottingham requested review from XDex and artur-ciocanu May 6, 2021 05:29
@coveralls
Copy link

Coverage Status

Coverage remained the same at 92.502% when pulling 6ae37fc on dcottingham:TNT-40793 into 630ec17 on adobe:main.

@artur-ciocanu
Copy link
Contributor

@dcottingham overall it looks, however I am not sure which breaking changes are you referring to. Is it Parameters and TraceResponse or something else?

Copy link
Contributor

@XDex XDex left a comment

Choose a reason for hiding this comment

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

@dcottingham Looks good, I'm not sure replacing Oneof... types with actual TS union types should break anything, it's actually much better to have union types than the old custom types..

@dcottingham
Copy link
Contributor Author

@dcottingham Looks good, I'm not sure replacing Oneof... types with actual TS union types should break anything, it's actually much better to have union types than the old custom types..

@XDex, Agreed it is definitely better. But I'm not sure if customers are using these TS interfaces explicitly in their code. If they are then they will have some very minor changes to make since we removed the Oneof... types. That's the only change that isn't fully backward compatible. And if customers are not explicitly using the Oneof... type then they won't even notice a difference.

CC: @artur-ciocanu

Copy link
Contributor

@artur-ciocanu artur-ciocanu left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@dcottingham dcottingham changed the title TNT-40793 - Regenerate openapi models TNT-40793, TNT-40657 - Regenerate openapi models May 10, 2021
@dcottingham dcottingham merged commit 22510a7 into adobe:main May 13, 2021
@dcottingham dcottingham deleted the TNT-40793 branch May 13, 2021 17:38
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.

4 participants