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: add more reusable objects to the components object #792

Conversation

magicmatatjahu
Copy link
Member

@magicmatatjahu magicmatatjahu commented May 10, 2022


title: add more reusable objects to the components object

That PR adds more more reusable objects to the components object like External Documentation Object and Tag Object.

Tags and External Documentation objects are used in several places and it would be a good solution to "convert" them to reusable objects, not inline.

Related issue(s)
Part of #750

PR for spec-json-schemas repo asyncapi/spec-json-schemas#245

@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
No Duplication information No Duplication information

spec/asyncapi.md Outdated
@@ -1439,11 +1439,15 @@ Field Name | Type | Description
---|:---|---
<a name="componentsSchemas"></a> schemas | Map[`string`, [Schema Object](#schemaObject) \| [Reference Object](#referenceObject)] | An object to hold reusable [Schema Objects](#schemaObject).
<a name="componentsServers"></a> servers | Map[`string`, [Server Object](#serverObject) \| [Reference Object](#referenceObject)] | An object to hold reusable [Server Objects](#serverObject).
<a name="componentsChannels"></a> channels | Map[`string`, [Channel Item Object](#channelItemObject)] | An object to hold reusable [Channel Item Objects](#channelItemObject).
<a name="componentsChannels"></a> channels | Map[`string`, [Channel Item Object](#channelItemObject)] \| [Reference Object](#referenceObject)] | An object to hold reusable [Channel Item Objects](#channelItemObject).
<a name="componentsOperations"></a> operations | Map[`string`, [Operation Item Object](#operationObject)] \| [Reference Object](#referenceObject)] | An object to hold reusable [Operation Item Objects](#operationObject).
Copy link
Member

Choose a reason for hiding this comment

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

@magicmatatjahu would you mind removing operations from this PR? I already moved them in this one: #806.

Copy link
Member Author

Choose a reason for hiding this comment

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

@fmvilas Done :)

@magicmatatjahu magicmatatjahu force-pushed the next-major/extend-components branch from bd12bd5 to 09b4e7f Compare June 27, 2022 11:12
@magicmatatjahu
Copy link
Member Author

@KhudaDad414 Could you check the failing Check Markdown links job? I guess that after merging it f269234 it should be fixed, but it's not. Please correct me if I'm wrong.

@smoya
Copy link
Member

smoya commented Jul 7, 2022

@magicmatatjahu Why this PR is targetting next-major-spec instead of next-spec? I can't see where the breaking change is (most probably missing something).
As a minor comment, the title prefix would need to be feat:.

spec/asyncapi.md Outdated
@@ -572,8 +572,8 @@ Field Name | Type | Description
<a name="channelItemObjectRef"></a>$ref | `string` | Allows for an external definition of this channel item. The referenced structure MUST be in the format of a [Channel Item Object](#channelItemObject). If there are conflicts between the referenced definition and this Channel Item's definition, the behavior is *undefined*. <br/><br/>**Deprecated:** Usage of the `$ref` property has been deprecated.
<a name="channelItemObjectDescription"></a>description | `string` | An optional description of this channel item. [CommonMark syntax](https://spec.commonmark.org/) can be used for rich text representation.
<a name="channelItemObjectServers"></a>servers | [`string`] | The servers on which this channel is available, specified as an optional unordered list of names (string keys) of [Server Objects](#serverObject) defined in the [Servers Object](#serversObject) (a map). If `servers` is absent or empty then this channel must be available on all servers defined in the [Servers Object](#serversObject).
<a name="channelItemObjectSubscribe"></a>subscribe | [Operation Object](#operationObject) | A definition of the SUBSCRIBE operation, which defines the messages produced by the application and sent to the channel.
<a name="channelItemObjectPublish"></a>publish | [Operation Object](#operationObject) | A definition of the PUBLISH operation, which defines the messages consumed by the application from the channel.
<a name="channelItemObjectSubscribe"></a>subscribe | [[Operation Object](#operationObject) | A definition of the SUBSCRIBE operation, which defines the messages produced by the application and sent to the channel.
Copy link
Member

Choose a reason for hiding this comment

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

Is this change needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Leftover of removing operations referencing from my PR. I will fix that, good catch!

@magicmatatjahu
Copy link
Member Author

@smoya I thought it better not to add it to 2.X.X but to 3.0.0 despite the lack of breaking changes. As for chore: I don't know how the release process will behave and don't want to release 3.0.0-prerelease1 😅

@smoya
Copy link
Member

smoya commented Jul 8, 2022

@smoya I thought it better not to add it to 2.X.X but to 3.0.0 despite the lack of breaking changes. As for chore: I don't know how the release process will behave and don't want to release 3.0.0-prerelease1 😅

Considering there is no BC and that it adds a really useful feature, I don't see any reason to not add this into 2.5 release that will happen in September.

@fmvilas
Copy link
Member

fmvilas commented Jul 11, 2022

While working on 3.0.0, we'll find a bunch of things that are not BC and could potentially be added to 2.x. However, IMHO, we must be careful not to overimprove 2.x because otherwise, there will be little incentive to migrate to 3.0.0 when it's done. This work was part of a 3.0.0 task and I'd consider it for 3.0.0 only.

@smoya
Copy link
Member

smoya commented Jul 11, 2022

While working on 3.0.0, we'll find a bunch of things that are not BC and could potentially be added to 2.x. However, IMHO, we must be careful not to overimprove 2.x because otherwise, there will be little incentive to migrate to 3.0.0 when it's done. This work was part of a 3.0.0 task and I'd consider it for 3.0.0 only.

Even though I'm afraid I have to disagree, I can say 👍 to both of you If you consider it is a good strategy to follow.

@jonaslagoni jonaslagoni mentioned this pull request Jul 18, 2022
@jonaslagoni
Copy link
Member

What can we do to move this forward, is it only reviews we need @magicmatatjahu ? 🙂

@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
No Duplication information No Duplication information

@magicmatatjahu
Copy link
Member Author

@jonaslagoni

What can we do to move this forward, is it only reviews we need @magicmatatjahu ? 🙂

Yes. Here is also PR for json-schemas asyncapi/spec-json-schemas#245

cc @derberg @fmvilas @dalelane

spec/asyncapi.md Outdated Show resolved Hide resolved
@magicmatatjahu magicmatatjahu force-pushed the next-major/extend-components branch from f6e9509 to 6c1d792 Compare October 3, 2022 09:43
@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 3, 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
No Duplication information No Duplication information

@magicmatatjahu
Copy link
Member Author

Rebased with current next-major-spec. Could you check again? @derberg @fmvilas @dalelane Thanks!

@derberg
Copy link
Member

derberg commented Oct 3, 2022

it is new feature, should be feat and RC should be released

@magicmatatjahu magicmatatjahu changed the title chore: add more reusable objects to the components object feat: add more reusable objects to the components object Oct 3, 2022
@magicmatatjahu
Copy link
Member Author

@derberg Right, changed. Thanks!

@magicmatatjahu
Copy link
Member Author

I think that it could be merged :) I also have PR in json-schemas asyncapi/spec-json-schemas#245 In ParserJS also asyncapi/parser-js#639 (still draft - without tests)

cc @derberg @fmvilas

@derberg
Copy link
Member

derberg commented Oct 11, 2022

/rtm

@asyncapi-bot asyncapi-bot merged commit a1783a2 into asyncapi:next-major-spec Oct 11, 2022
@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 3.0.0-next-major-spec.3 🎉

The release is available on GitHub release

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.

7 participants