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 object descriptions and examples #425

Merged
merged 52 commits into from
Nov 21, 2023

Conversation

AceTheCreator
Copy link
Member

This PR adds descriptions and examples to version 3.0.0 of the Schema

cc @derberg @jonaslagoni

@derberg
Copy link
Member

derberg commented Sep 28, 2023

@AceTheCreator the quality of this PR is questionable 😆

@jonaslagoni
Copy link
Member

@AceTheCreator you are targeting the wrong branch I think 🤔

@AceTheCreator AceTheCreator changed the base branch from master to next-major-spec September 28, 2023 08:45
@AceTheCreator
Copy link
Member Author

@AceTheCreator the quality of this PR is questionable 😆

In what way? 😅

@AceTheCreator
Copy link
Member Author

@AceTheCreator you are targeting the wrong branch I think 🤔

You're right 😄

@derberg
Copy link
Member

derberg commented Sep 28, 2023

@AceTheCreator there was a lot of strange commits, over 150 files modified, and sonar failing 😛 but now I see some stuff was related to wrong target branch

still you have some unrelated changes:

  • why changes in go files?
  • remove DS_Store stuff

@AceTheCreator
Copy link
Member Author

Updated the PR @derberg

@derberg
Copy link
Member

derberg commented Oct 4, 2023

did you follow https://github.com/asyncapi/spec-json-schemas#schemastore-compatibility-testing procedure locally to make sure new descriptions and examples are good and display properly in IDE?

@AceTheCreator
Copy link
Member Author

did you follow https://github.com/asyncapi/spec-json-schemas#schemastore-compatibility-testing procedure locally to make sure new descriptions and examples are good and display properly in IDE?

I tested it locally, and it works as expected, but I noticed the examples property doesn't display well. But keep in mind that it's the same for version 2.6.0, which means that the yaml-server-language hover feat that helps prettify example objects is broken somehow. I opened an issue there, and you can find it here redhat-developer/yaml-language-server#928

@derberg derberg changed the title feat: added object descriptions and examples feat: add object descriptions and examples Oct 11, 2023
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.

awesome, just remove the last .DS_Store and we can merge

"type": "string"
}
"type": "string",
"description": "A short description for security scheme."
Copy link
Member

@smoya smoya Oct 11, 2023

Choose a reason for hiding this comment

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

Not sure how this is handled at CI but this indentation is wrong. There are others as well, so please check 🙏

@AceTheCreator
Copy link
Member Author

Conflict resolved!

cc @derberg

Copy link

sonarcloud bot commented Nov 16, 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

@derberg derberg requested a review from smoya November 16, 2023 15:16
Copy link
Collaborator

@char0n char0n left a comment

Choose a reason for hiding this comment

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

Hi @AceTheCreator,

After I've realized that custom dialect example keyword is later translated by tooling to examples keyword, the PR looks good to me. Nice job!

One thing I've noticed is that references are being made to http://asyncapi.com/definitions/3.0.0/ReferenceObject.json, but file with this name doesn't exist. Nevertheless referencing ReferenceObject.json existed in the code before your PR, so just pointing it out.

@AceTheCreator
Copy link
Member Author

AceTheCreator commented Nov 17, 2023

One thing I've noticed is that references are being made to http://asyncapi.com/definitions/3.0.0/ReferenceObject.json, but file with this name doesn't exist. Nevertheless referencing ReferenceObject.json existed in the code before your PR, so just pointing it out.

Do you mean the URL? if yes, then it's because we are still on the next-major-spec branch and not the master(at least I think). So the /3.0.0 isn't available yet.

thoughts @derberg ?

Copy link
Member

derberg commented Nov 17, 2023

yup, entire 3.0 is on feature branch, so after release/merge, these links will work just fine, I think 😃

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!

@derberg
Copy link
Member

derberg commented Nov 21, 2023

/rtm

@AceTheCreator thanks mate, this is huge!!!

@asyncapi-bot asyncapi-bot merged commit 75459f2 into asyncapi:next-major-spec Nov 21, 2023
11 checks passed
@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 6.0.0-next-major-spec.14 🎉

The release is available on:

Your semantic-release bot 📦🚀

@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 6.0.0-next-major-spec.15 🎉

The release is available on:

Your semantic-release bot 📦🚀

@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 6.0.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.

7 participants