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

fix(ibmmq): make topic field optional #176

Merged
merged 2 commits into from
Jan 16, 2023
Merged

fix(ibmmq): make topic field optional #176

merged 2 commits into from
Jan 16, 2023

Conversation

char0n
Copy link
Collaborator

@char0n char0n commented Dec 20, 2022 •
edited
Loading

When destinationType=topic, queue field MUST NOT be present and topic MAY be defined.

Refs #169

This change is a reaction to #169 (comment) and #169 (comment)

@char0n
fix(ibmmq): make topic field optional
23cfa9a 
When destinationType=topic, queue field MUST NOT be present
and topic may be defined.

Refs asyncapi#169
@char0n char0n mentioned this pull request Dec 20, 2022
Closed
@char0n
Copy link
Collaborator Author

char0n commented Dec 20, 2022

@rcoppen please have a look at this as well.

@derberg
Copy link
Member

derberg commented Jan 16, 2023

@KhudaDad414 as it involves json schemas, you need to approve too

@jonaslagoni yo, pinging you so you know to update asyncapi/spec-json-schemas#239

@derberg
Copy link
Member

derberg commented Jan 16, 2023

@KhudaDad414 and yeah, when you approve, please merge

@KhudaDad414
Copy link
Member

/rtm

@asyncapi-bot asyncapi-bot merged commit c175a70 into asyncapi:master Jan 16, 2023
@jonaslagoni
Copy link
Member

No binding version change? 🤔

KhudaDad414 added a commit that referenced this pull request Jan 16, 2023
@KhudaDad414
Revert "fix(ibmmq): make topic field optional (#176)"
2bc9498 
This reverts commit c175a70.
@KhudaDad414
Copy link
Member

@jonaslagoni should we revert it and merge it with the version change? 🤔

@jonaslagoni
Copy link
Member

It's backward compatible, but it will be like the required version never existed, so in the optimal world I would say yes 😄

@derberg
Copy link
Member

derberg commented Jan 16, 2023

@KhudaDad414 @jonaslagoni but this was change in the schema only. The binding spec is correct and says optional, just the schema mistakenly required it. So it is bug in schema, not really a change in a spec. So IMHO no version bump

@jonaslagoni
Copy link
Member

Ahh, okay 👍

@jonaslagoni
Copy link
Member

I will update the schema once asyncapi/spec-json-schemas#239 is merged.

@char0n char0n mentioned this pull request Feb 6, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KhudaDad414 KhudaDad414 KhudaDad414 approved these changes

@rcoppen rcoppen rcoppen approved these changes

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

Successfully merging this pull request may close these issues.
6 participants
@char0n @derberg @KhudaDad414 @jonaslagoni @rcoppen @asyncapi-bot