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

chore(blog): the reference rabbit hole #461

Merged
merged 32 commits into from
Nov 30, 2021

Conversation

jonaslagoni
Copy link
Member

@jonaslagoni jonaslagoni commented Nov 15, 2021

Description
This blog post explains why and how the following issues came to be:

  • spec #650, highlight the discrepancies when the Reference Object can be used.
  • spec #649, tries to solve the core issue that $ref means two different things, depending on when it's used.
  • spec #655, what do you do when encountering $schema and Message Object schemaFormat
  • parser-js #405, highlight that the parser accurately validates incorrect AsyncAPI documents, because it bundles references before validating.
  • parser-js #404, highlight that the parser allows for keywords to be defined together with $ref and are not being ignored.
  • parser-js #403, highlight that the parser does not care about $id in the Schema Object when it should.

@netlify
Copy link

netlify bot commented Nov 15, 2021

✔️ Deploy Preview for asyncapi-website ready!
Built without sensitive environment variables

🔨 Explore the source changes: 8afcb41

🔍 Inspect the deploy log: https://app.netlify.com/sites/asyncapi-website/deploys/61a5fb86af780b0007550239

😎 Browse the preview: https://deploy-preview-461--asyncapi-website.netlify.app

@jonaslagoni jonaslagoni marked this pull request as ready for review November 15, 2021 16:55
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.

Few nitpicks, some more useful than others to me, especially for readability.
Looks cool otherwise ! 👍

pages/blog/the-reference-rabbit-hole.md Outdated Show resolved Hide resolved
pages/blog/the-reference-rabbit-hole.md Outdated Show resolved Hide resolved
pages/blog/the-reference-rabbit-hole.md Outdated Show resolved Hide resolved
pages/blog/the-reference-rabbit-hole.md Outdated Show resolved Hide resolved
pages/blog/the-reference-rabbit-hole.md Outdated Show resolved Hide resolved
pages/blog/the-reference-rabbit-hole.md Outdated Show resolved Hide resolved
pages/blog/the-reference-rabbit-hole.md Outdated Show resolved Hide resolved
pages/blog/the-reference-rabbit-hole.md Outdated Show resolved Hide resolved
@jonaslagoni
Copy link
Member Author

Removing request for review for a second, need to rewrite parts of it.

@jonaslagoni jonaslagoni marked this pull request as draft November 17, 2021 17:26
@jonaslagoni jonaslagoni marked this pull request as ready for review November 18, 2021 19:59
@jonaslagoni
Copy link
Member Author

Okay, I think it is good shape now 😅 Sorry for making you double review it @smoya 🙇

Copy link
Member

@quetzalliwrites quetzalliwrites left a comment

Choose a reason for hiding this comment

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

Sorry @jonaslagoni !! I thought I had finished the review, now I see I left the comments pending! smh 🤦‍♀️

Ok, doing it right now :)

pages/blog/the-reference-rabbit-hole.md Outdated Show resolved Hide resolved
pages/blog/the-reference-rabbit-hole.md Outdated Show resolved Hide resolved
pages/blog/the-reference-rabbit-hole.md Outdated Show resolved Hide resolved
pages/blog/the-reference-rabbit-hole.md Outdated Show resolved Hide resolved
pages/blog/the-reference-rabbit-hole.md Show resolved Hide resolved
pages/blog/the-reference-rabbit-hole.md Outdated Show resolved Hide resolved
pages/blog/the-reference-rabbit-hole.md Outdated Show resolved Hide resolved
pages/blog/the-reference-rabbit-hole.md Outdated Show resolved Hide resolved
pages/blog/the-reference-rabbit-hole.md Outdated Show resolved Hide resolved
pages/blog/the-reference-rabbit-hole.md Outdated Show resolved Hide resolved
@quetzalliwrites
Copy link
Member

@derberg I have left my review successfully this time 😀

@jonaslagoni
Copy link
Member Author

Quality review as always @alequetzalli @smoya, addressed all your comments 🙂

smoya
smoya previously approved these changes Nov 29, 2021
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!! 🚀 🌔

Copy link
Member

@quetzalliwrites quetzalliwrites left a comment

Choose a reason for hiding this comment

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

LGMT 🚀 🌟

@derberg derberg dismissed stale reviews from quetzalliwrites and smoya via a77862a November 30, 2021 08:08
derberg
derberg previously approved these changes Nov 30, 2021
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.

review of @smoya and @alequetzalli were dismissed but only because I pushed publish date update, so I approve and we can merge. We release this article today

@derberg
Copy link
Member

derberg commented Nov 30, 2021

@jonaslagoni before we merge, are you 100% sure about the article title? me myself and I bet others too, decide to click on a link and read an article when I more or less know what it is about. Tbh The Reference Rabbit Hole tells me nothing. Wouldn't it be better to have The JSON Reference ($ref) Rabbit Hole?

@jonaslagoni
Copy link
Member Author

@jonaslagoni before we merge, are you 100% sure about the article title? me myself and I bet others too, decide to click on a link and read an article when I more or less know what it is about. Tbh The Reference Rabbit Hole tells me nothing. Wouldn't it be better to have The JSON Reference ($ref) Rabbit Hole?

Not sure, hate figuring out titles, this one just seemed natural. Especially since it is not really only about JSON Schema reference, but both 🤔 Sooo, not sure 😅

@derberg
Copy link
Member

derberg commented Nov 30, 2021

@jonaslagoni leaving it up to you, just tell me if I should release or not. Just sharing it on reddit or hackernews with this title will not increase much reach IMHO, which is fine if the article is purely for AsyncAPI community only

@smoya
Copy link
Member

smoya commented Nov 30, 2021

@jonaslagoni before we merge, are you 100% sure about the article title? me myself and I bet others too, decide to click on a link and read an article when I more or less know what it is about. Tbh The Reference Rabbit Hole tells me nothing. Wouldn't it be better to have The JSON Reference ($ref) Rabbit Hole?

Not sure, hate figuring out titles, this one just seemed natural. Especially since it is not really only about JSON Schema reference, but both 🤔 Sooo, not sure 😅

What about something more descriptive then, something in the style of "JSON Schema $ref is not JSON Reference" (shouldnt be this one, it is just an example of the style)

@derberg
Copy link
Member

derberg commented Nov 30, 2021

yo, I now read the article. Sorry, but tbh it is super specific to AsyncAPI, very technical and requires deep AsyncAPI understanding, so the title can be as it is as I don't see a point of putting it on reddit or hackernews but only on official AsyncAPI social accounts and their followers.

Just please fix the broken link and I think we can go:

Screenshot 2021-11-30 at 10 48 48

@jonaslagoni
Copy link
Member Author

@derberg done.

@jonaslagoni jonaslagoni requested review from derberg and removed request for smoya November 30, 2021 10:27
@derberg derberg merged commit 72a254c into asyncapi:master Nov 30, 2021
@jonaslagoni jonaslagoni deleted the feature/reference_rabbithole branch April 11, 2022 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants