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.

Sign up for GitHub

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

Jump to bottom

feat: add Avro 1.9 Schema #363

Merged
merged 4 commits into from
Apr 25, 2023
Merged

feat: add Avro 1.9 Schema #363

merged 4 commits into from
Apr 25, 2023

Conversation

jonaslagoni
Copy link
Member

@jonaslagoni jonaslagoni commented Mar 16, 2023 •
edited
Loading

Description
As done in #347 this PR adds the Avro Schema object for all v2 versions.

It is quite a few duplicate schemas... But there is currently no setup to share schemas between versions... Would be great to have at some point. See #364

derberg
derberg reviewed Mar 30, 2023
View reviewed changes
Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

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

yeah, I wonder how is tooling support handling if/then 😅

@dalelane @M3lkior this PR means that basically Avro schema parser do not need special validation as it could be handled by main JSON Schema. The only thing is that probably 1.8.2 should be added to spec and here 🤔

@jonaslagoni
Copy link
Member Author

yeah, I wonder how is tooling support handling if/then 😅

Which tooling? 😄

If they dont support it, it does not really matter, because then we are back to whats currently in place, where it can be any.

dalelane
dalelane approved these changes Apr 6, 2023
View reviewed changes
Copy link
Collaborator

@dalelane dalelane left a comment

Choose a reason for hiding this comment

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

LGTM - as highlighted, the duplication is unfortunate but something we can hopefully improve in future

@jonaslagoni
Copy link
Member Author

/rtm

@asyncapi-bot
Copy link
Contributor

Hello, @jonaslagoni! 👋🏼
This PR is not up to date with the base branch and can't be merged.
Please update your branch manually with the latest version of the base branch.
PRO-TIP: Add a comment to your PR with the text: /au or /autoupdate and our bot will take care of updating the branch in the future. The only requirement for this to work is to enable Allow edits from maintainers option in your PR.
Thanks 😄

@sonarqubecloud
Copy link

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

@derberg
Copy link
Member

derberg commented Apr 25, 2023

/rtm

@asyncapi-bot asyncapi-bot merged commit b8cd742 into asyncapi:master Apr 25, 2023
@derberg derberg mentioned this pull request Apr 25, 2023
Merged
@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 4.3.0 🎉

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
Reviewers

@derberg derberg derberg approved these changes

@dalelane dalelane dalelane approved these changes

@fmvilas fmvilas Awaiting requested review from fmvilas fmvilas is a code owner

@smoya smoya Awaiting requested review from smoya smoya is a code owner

@char0n char0n Awaiting requested review from char0n char0n is a code owner

@asyncapi-bot-eve asyncapi-bot-eve Awaiting requested review from asyncapi-bot-eve asyncapi-bot-eve is a code owner

Assignees
No one assigned
Labels
ready-to-merge released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jonaslagoni @asyncapi-bot @derberg @dalelane