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: schema type casing #221

Merged
merged 3 commits into from
Jun 22, 2022
Merged

fix: schema type casing #221

merged 3 commits into from
Jun 22, 2022

Conversation

alexstaroselsky
Copy link
Contributor

Update logic for type application casing to ensure lowercase.

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Test
  • Docs
  • Refactor
  • Build-related changes
  • Other, please describe:

Description:

When types such as Array<Song> or Song[] are used, the type for the property resolves to 'Array' rather than the officially support OpenAPI data type of 'array'. When the type is 'Array', the OpenAPI document does not correctly recognize the property as a valid array. The officially supported types are lowercase anyway:

Update logic for type application casing to ensure lowercase.
@alexstaroselsky
Copy link
Contributor Author

Found another location of type.expression.name in getSchema() will update that as well and update relevant tests to demonstrate fix.

Update logic for type application casing to ensure lowercase.
Update logic for type application casing to ensure lowercase.
@sonarcloud
Copy link

sonarcloud bot commented Jun 14, 2022

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
2.4% 2.4% Duplication

@alexstaroselsky
Copy link
Contributor Author

alexstaroselsky commented Jun 15, 2022

@kevinccbsg or @bri06 or @LonelyPrincess Could you please clarify who would be a good member to review this PR? Or if anything additionally is needed for it to be ready for review? Thanks.

@LonelyPrincess
Copy link
Contributor

@kevinccbsg or @bri06 or @LonelyPrincess Could you please clarify who would be a good member to review this PR? Or if anything additionally is needed for it to be ready for review? Thanks.

This looks fine to me! The PR description is pretty clear, and doesn't seem like we would be missing anything to consider it complete. I've approved it, although will wait for @kevinccbsg to give the OK before merging. Thanks for your contribution! 😉

@kevinccbsg
Copy link
Member

Thanks for reviewing @LonelyPrincess !! @alexstaroselsky we will do a relases asap.

@kevinccbsg
Copy link
Member

@all-contributors please add @alexstaroselsky for code

@allcontributors
Copy link
Contributor

@kevinccbsg

I've put up a pull request to add @alexstaroselsky! 🎉

@alexstaroselsky
Copy link
Contributor Author

Thanks all for helping get this through. @kevinccbsg can you confirm when this would be tentatively scheduled for release? If there are other features waiting for review/approval that would delay it, could there be a beta publish for testing?

@kevinccbsg
Copy link
Member

@alexstaroselsky sorry for the delay Today we will do a release.

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.

None yet

3 participants