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: parameter conversion for v3 #190

Merged
merged 4 commits into from
Jul 24, 2023

Conversation

jonaslagoni
Copy link
Member

Description
This PR fixes the last problem, parameter conversion to the new object that was left over from #180

Related issue(s)
Fixes #110

derberg
derberg previously approved these changes Jul 24, 2023
@jonaslagoni
Copy link
Member Author

That was quick, thanks @derberg 🙇

@jonaslagoni
Copy link
Member Author

/rtm

src/third-version.ts Outdated Show resolved Hide resolved
src/third-version.ts Outdated Show resolved Hide resolved
Co-authored-by: Maciej Urbańczyk <[email protected]>
Co-authored-by: Maciej Urbańczyk <[email protected]>
@sonarcloud
Copy link

sonarcloud bot commented Jul 24, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@jonaslagoni
Copy link
Member Author

Nice catch @magicmatatjahu 😆

@magicmatatjahu
Copy link
Member

I wonder how to treat all stuff like minLength etc in schema, as extension with x- prefix or skip/leave it?

@jonaslagoni
Copy link
Member Author

jonaslagoni commented Jul 24, 2023

I wonder how to treat all stuff like minLength etc in schema, as extension with x- prefix or skip/leave it?

@magicmatatjahu I would say we should make the users decide that, see if anyone complains down the road.

If we receive complains, then it might be that it should be included in the spec as well, not explicitly in the converter. Because that means users use them, when we assumed they did not.

Or we can always add it as an option or something else.

@magicmatatjahu
Copy link
Member

@jonaslagoni Sure, I agree, also we can even add option to convert function to leave original schema as x-original-schema extension

@jonaslagoni
Copy link
Member Author

Also an option yea 👍

But the spec assumes these properties are not used at all (why they are not present in the new parameter object), so I don't think it makes sense to assume it here in the tool 🙂

@jonaslagoni
Copy link
Member Author

Mind making the final review @magicmatatjahu? 🙏

Copy link
Member

@magicmatatjahu magicmatatjahu left a comment

Choose a reason for hiding this comment

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

LGTM!

@jonaslagoni
Copy link
Member Author

Thank you @magicmatatjahu 🙇

@jonaslagoni
Copy link
Member Author

/rtm

@asyncapi-bot asyncapi-bot merged commit 35f3dca into asyncapi:master Jul 24, 2023
11 checks passed
@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 1.3.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support converting to 3.0.0
4 participants