-
-
Notifications
You must be signed in to change notification settings - Fork 21
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
fix: parameter conversion for v3 #190
Conversation
That was quick, thanks @derberg 🙇 |
/rtm |
Co-authored-by: Maciej Urbańczyk <[email protected]>
Co-authored-by: Maciej Urbańczyk <[email protected]>
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
Nice catch @magicmatatjahu 😆 |
I wonder how to treat all stuff like |
@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. |
@jonaslagoni Sure, I agree, also we can even add option to convert function to leave original schema as |
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 🙂 |
Mind making the final review @magicmatatjahu? 🙏 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Thank you @magicmatatjahu 🙇 |
/rtm |
🎉 This PR is included in version 1.3.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Description
This PR fixes the last problem, parameter conversion to the new object that was left over from #180
Related issue(s)
Fixes #110