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

♻️ Remove modify id script in favor of openapi-ts config #1434

Merged
merged 1 commit into from
Nov 27, 2024

Conversation

patrick91
Copy link
Contributor

@patrick91 patrick91 commented Nov 25, 2024

This PR removes the modify-id script; this is now handled by a custom config for openapi-ts.

The behaviour is slightly different, as we are now only renaming the methods and not the types, which is better in cases where we have operations with the same name; see #1429's create_user for an example :)

Base automatically changed from feature/update-openapi-ts to master November 27, 2024 11:07
@patrick91 patrick91 force-pushed the feature/remove-modify-id-script branch from 4e0ed8f to bda9841 Compare November 27, 2024 11:16
@tiangolo
Copy link
Member

The behaviour is slightly different, as we are now only renaming the methods and not the types, which is better in cases where we have operations with the same name; see #1429 create_user for an example :)

Interesting, I think this is just from the version upgrade. 🤔 The current/old script only renamed the path operations, not really the types. I suspect they realized that the generated types needed to be disambiguated maybe. 🤔

Copy link
Member

@tiangolo tiangolo 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! Thank you! 🚀 🎉

@tiangolo tiangolo changed the title ♻️ Remove modify id script in favour of openapi-ts config ♻️ Remove modify id script in favor of openapi-ts config Nov 27, 2024
@tiangolo tiangolo merged commit 953e9e5 into master Nov 27, 2024
15 checks passed
@tiangolo tiangolo deleted the feature/remove-modify-id-script branch November 27, 2024 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants