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

docs: adding content for module 3 #9

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

TRohit20
Copy link

@TRohit20 TRohit20 commented Dec 2, 2023

Description
As part of creating a 100-level interactive learning path for AsyncAPI, adding the content for module 3.

Outline for the content to be present in the module from this pull request:

Module-3: AsyncAPI Specification

  • Understanding the structure of an AsyncAPI document.
  • request/reply

Spaceship fixed, we see Chan leaving to his planet on his spaceship

@TRohit20 TRohit20 marked this pull request as draft December 2, 2023 18:08
@quetzalliwrites
Copy link
Member

Hey folks, so please update your planet names to:

  • Eve's planet: Capuccinova
  • Chan's planet: Brownieterra

@TRohit20 TRohit20 marked this pull request as ready for review December 21, 2023 14:55
@quetzalliwrites
Copy link
Member

Heyo @smoya, do you think Rohit's updates look correct now? If you approve his content, then we don't need @derberg's review/time.

Or let us know if you still don't approve it and what you want us to fix 😺

100-level-AsyncAPI-Intro/step2.md Outdated Show resolved Hide resolved
@smoya
Copy link
Member

smoya commented Jan 29, 2024

Or let us know if you still don't approve it and what you want us to fix 😺

Second round done!

@smoya
Copy link
Member

smoya commented Jan 29, 2024

@TRohit20 I noticed the PR description mentions you want to let the users understand what/which are the basic? components of an AsyncAPI document, including a request/reply pattern demo.
Besides showing an example, wouldn't be necessary to make an intro to what are the basic components? Like what is a channel or a message, a server, etc?
cc @alequetzalli

@mhmohona
Copy link
Contributor

mhmohona commented Jan 29, 2024

Hello @smoya, we have those basic content covered in #5.

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.

Looks good to me, thank you @smoya for the last feedback item. As Mahfuza noted, that information will be covered in the next module 👍🏻

Do you think it looks good to you too now? 😸

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! 🚀 🌔

@TRohit20
Copy link
Author

🚀

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.

I withdraw my approval since I noticed there are 3 PR's opened pointing to the exact same file step2.md.

Please unify all of those into one single PR so we can review the file as a whole.

cc @TRohit20 @mhmohona @alequetzalli

@TRohit20
Copy link
Author

@smoya As per your suggestion above, I have now merged the content from the other PRs into this PR(#9). This PR now covers the content for the entirety of module-3. We are waiting on your final technical review now😄

Thank you!

cc @alequetzalli @mhmohona

@TRohit20 TRohit20 requested a review from smoya February 14, 2024 07:08
@quetzalliwrites
Copy link
Member

thank you for taking care of this quickly @TRohit20! appreciate your commitment 🐻

roomidchat:
address: chat/{roomId}
messages:
chatMessage:
Copy link
Member

Choose a reason for hiding this comment

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

You are describing the message and its payload here. However, in the rest of the examples, both are refered from components.
Maybe makes sense to unify those and use references here as well.

cc @alequetzalli

Copy link
Member

Choose a reason for hiding this comment

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

thank you for pointing this out, @smoya!

@smoya
Copy link
Member

smoya commented Feb 19, 2024

1st review round done @TRohit20 @alequetzalli

@quetzalliwrites
Copy link
Member

FYI: I removed the exercises in this module because their addition here is a mistake... and as Sergio noticed, some of the content in module 4 now looked duplicate to module 3. The original goal was always to keep the exercises in module 4, so adding exercises to this module 3 is not following our 100-level intro to AsyncAPI learning path roadmap.

cc @smoya @TRohit20 @mhmohona

Screenshot 2024-02-20 at 2 19 33 PM

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.

several technical issues to address and code snippets to correct

@TRohit20
Copy link
Author

@mhmohona Could you kindly look into the suggestions made by Sergio as part of the technical review and make the necessary changes through commit suggestions at the earliest?

Thank you!

100-level-AsyncAPI-Intro/step2.md Outdated Show resolved Hide resolved
100-level-AsyncAPI-Intro/step2.md Outdated Show resolved Hide resolved
100-level-AsyncAPI-Intro/step2.md Outdated Show resolved Hide resolved
100-level-AsyncAPI-Intro/step2.md Outdated Show resolved Hide resolved
100-level-AsyncAPI-Intro/step2.md Show resolved Hide resolved
100-level-AsyncAPI-Intro/step2.md Outdated Show resolved Hide resolved
100-level-AsyncAPI-Intro/step2.md Outdated Show resolved Hide resolved
@TRohit20
Copy link
Author

I have committed all the changes made by @mhmohona based on @smoya technical review, all suggested changes and issues seem to be addressed, which is why I would like to request Sergio for another technical review to check if the content for module 3 is now ready to go.

cc @alequetzalli

Copy link

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 ❤️

@github-actions github-actions bot added the stale label Jun 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants