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

feat: remove messageId when converting to 3.0.0 #202

Merged
merged 1 commit into from
Nov 7, 2023

Conversation

fmvilas
Copy link
Member

@fmvilas fmvilas commented Nov 6, 2023

Description

Removes the messageId property from the Message objects.

Related issue(s)
asyncapi/spec#978

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

Copy link

sonarqubecloud bot commented Nov 6, 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

Copy link
Member

@smoya smoya left a comment

Choose a reason for hiding this comment

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

Shall we add a warning or similar mentioning messageId will be "lost" and recommend changing the key of the map to the value messageId had?

@fmvilas
Copy link
Member Author

fmvilas commented Nov 6, 2023

changing the key of the map to the value messageId had

@smoya isn't it what's already happening? 🤔 I mean, before we used to have oneOf with an array. Now this oneOf is replaced with an object where the keys are the message ids, and we're applying the messageId value when it exists.

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.

This PR only removes messageId when it is no longer needed, but still, if id is present, it is used as a key for a message in v3. So lgtm

@smoya
Copy link
Member

smoya commented Nov 6, 2023

changing the key of the map to the value messageId had

@smoya isn't it what's already happening? 🤔 I mean, before we used to have oneOf with an array. Now this oneOf is replaced with an object where the keys are the message ids, and we're applying the messageId value when it exists.

Oh yeah, by removing the messageId the right ID will be picked up 👍 gotcha

Copy link
Member

@smoya smoya left a comment

Choose a reason for hiding this comment

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

LGTM!

@fmvilas
Copy link
Member Author

fmvilas commented Nov 7, 2023

/rtm

@asyncapi-bot asyncapi-bot merged commit 63547e6 into asyncapi:master Nov 7, 2023
20 checks passed
@fmvilas fmvilas deleted the remove-messageid branch November 7, 2023 10:33
@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 1.4.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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants