-
-
Notifications
You must be signed in to change notification settings - Fork 281
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
docs: update spec/asyncapi.md #595
Conversation
- correct article agreement - correct punctuation - correct grammar - reduce verbiage - use active voice vs. passive voice to increase readability - correct spelling
There was a problem hiding this 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 and the instructions about a basic recommended setup 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.
Kudos, SonarCloud Quality Gate passed! |
This pull request has been automatically marked as stale because it has not had recent activity 😴 It will be closed in 120 days if no further activity occurs. To unstale this pull request, add a comment with detailed explanation. There can be many reasons why some specific pull request has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model. Let us figure out together how to push this pull request forward. Connect with us through one of many communication channels we established here. Thank you for your patience ❤️ |
@alequetzalli hey, can I ask you for a favor? can you have a look at the proposed changes? I think they are correct and make sense but I would like expert opinion to make sure these changes do not imply different meaning of changed text 😅 |
@@ -144,7 +144,7 @@ By convention, the AsyncAPI Specification (A2S) file is named `asyncapi.json` or | |||
#### <a name="A2SObject"></a>AsyncAPI Object | |||
|
|||
This is the root document object for the API specification. | |||
It combines resource listing and API declaration together into one document. | |||
It combines resource listing and API declaration into one document. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It combines resource listing and API declaration into one document. | |
It combines the resource listing and API declaration into one document. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@derberg Just found one tiny grammar thing and the rest LGTM 👍🏽
@vladdoster can you have a look at suggestion from @alequetzalli and solve conflicts and then we can merge |
This pull request has been automatically marked as stale because it has not had recent activity 😴 It will be closed in 120 days if no further activity occurs. To unstale this pull request, add a comment with detailed explanation. There can be many reasons why some specific pull request has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model. Let us figure out together how to push this pull request forward. Connect with us through one of many communication channels we established here. Thank you for your patience ❤️ |
@vladdoster are you willing to continue with this PR? As @derberg mentioned there are some action items for this PR to proceed further:
|
@char0n Yes, I will apply the suggested change and squash commits. Sorry for delay. |
@vladdoster cool. Along with suggested change, conflicts needs to resolved as well. |
The patch version will not be considered by tooling, making no distinction between `1.0.0` and `1.0.1`. | ||
|
||
In subsequent versions of the AsyncAPI Specification, care will be given such that increments of the `minor` version should not interfere with operations of tooling developed to a lower minor version. Thus a hypothetical `1.1.0` specification should be usable with tooling designed for `1.0.0`. | ||
|
||
#### <a name="A2SIdString"></a>Identifier | ||
|
||
This field represents a unique universal identifier of the [application](#definitionsApplication) the AsyncAPI document is defining. It must conform to the URI format, according to [RFC3986](https://tools.ietf.org/html/rfc3986). | ||
This field represents a unique universal identifier of the [application](#definitionsApplication) the AsyncAPI document defines. It must conform to the URI format, according to [RFC3986](http://tools.ietf.org/html/rfc3986). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why change https://tools.ietf.org/html/rfc3986 to http://tools.ietf.org/html/rfc3986 ?
I know the http URL redirects to the https one anyway, so it doesn't really matter - but I still think it's better to treat the https one as canonical and link to that.
|
||
* User/Password. | ||
* API key (either as user or as password). | ||
* API key (either as a user or password). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not really sure what "either as a user or password" means here... that isn't specific to your change, I wasn't sure what it meant before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of these look like useful improvements, thanks! I just had a couple of questions which I've added as comments.
@vladdoster do you want to continue with this one? there are some important questions from @dalelane |
@vladdoster do you plan to continue with this one or shall we ask other contributors to take over? |
Since @vladdoster does not answer anymore, I plan to close this PR and instead open an issue so someone can pick it up from where it was left. Maybe marked as a good first issue btw. |
Makes sense to me 👍 |
I see you @derberg closed this PR. Are you planning to open a new issue as I suggested we could do? |
title: "update
spec/asyncapi.md
"Related issue(s):
N/A