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: new concept doc (application) #992

Merged
merged 34 commits into from
Dec 6, 2022

Conversation

quetzalliwrites
Copy link
Member

@quetzalliwrites quetzalliwrites commented Sep 29, 2022

This resolves #976

@quetzalliwrites quetzalliwrites added 📑 docs area/docs Specify what technical area given issue relates to. Its goal is to ease filtering good first issues. gsod This label should be used for issues or discussions related to ideas for Google Season of Docs labels Sep 29, 2022
@quetzalliwrites quetzalliwrites self-assigned this Sep 29, 2022
@netlify
Copy link

netlify bot commented Sep 29, 2022

Deploy Preview for asyncapi-website ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 36879a5
🔍 Latest deploy log https://app.netlify.com/sites/asyncapi-website/deploys/638efa941d84c60009a00802
😎 Deploy Preview https://deploy-preview-992--asyncapi-website.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@github-actions
Copy link

github-actions bot commented Sep 29, 2022

⚡️ Lighthouse report for the changes in this PR:

Category Score
🔴 Performance 49
🟠 Accessibility 88
🟠 Best practices 83
🟢 SEO 100
🔴 PWA 30

Lighthouse ran on https://deploy-preview-992--asyncapi-website.netlify.app/

@quetzalliwrites
Copy link
Member Author

Heyoooo, I have nooo idea if I got everything right here or not, so thank you for the review! (can't wait for ALL THE FEEDBACK 😂😂😂😂)

✌🏽 @smoya @derberg @fmvilas

@quetzalliwrites
Copy link
Member Author

quetzalliwrites commented Sep 29, 2022

@derberg Should I change the mermaid diagram to be TD so that the words don't get cut off like they currently do? 😢

Preview link:
https://deploy-preview-992--asyncapi-website.netlify.app/docs/concepts/application

Screen Shot 2022-09-29 at 1 48 00 PM

@quetzalliwrites
Copy link
Member Author

@fmvilas pingy pongo? 😂😄😬✌🏽

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.

nice work 👏🏼
one comment we need to discuss

pages/docs/concepts/application.md Outdated Show resolved Hide resolved
pages/docs/concepts/application.md Outdated Show resolved Hide resolved
quetzalliwrites and others added 3 commits October 17, 2022 17:08
Co-authored-by: Lukasz Gornicki <lpgornicki@gmail.com>
Co-authored-by: Lukasz Gornicki <lpgornicki@gmail.com>
@quetzalliwrites
Copy link
Member Author

quetzalliwrites commented Oct 18, 2022

Heyooo @derberg, sorry if I accidentally ask you to repeat yourself 😂

What else do I need to do on this doc? (I accepted your commit suggestions and I understood why going into Channel bindings was too much too soon. I just now felt confused about what else I should address in this PR 😅 )

Should I perhaps add a sentence like this?

"We can also see how a Message(/docs/concepts/message) is traveling through a medium called a channel(/docs/concepts/channel)."

thank you for the feedback so far!!

@derberg
Copy link
Member

derberg commented Oct 18, 2022

it looks good

I'm only thinking that we should extend this doc and also talk about application in context of AsyncAPI spec.

we could add section where we would write:

AsyncAPI document describes what a user can do with the application, and not what the application does. In other words, if your application is a producer, your AsyncAPI document should describe where users can subscribe.

makes sense? inspired by https://www.asyncapi.com/blog/publish-subscribe-semantics


also pinging all maintainers of the spec: @smoya @fmvilas @dalelane @char0n as all concept docs relate to spec 💯 and they should be aware and be able to suggest changes if needed

@quetzalliwrites quetzalliwrites dismissed stale reviews from smoya and derberg via 07528b5 November 7, 2022 20:58
@quetzalliwrites
Copy link
Member Author

@derberg 🚨 FYI, I want to merge this AT THE SAME TIME as when we merge @nelsonmic's protocol doc because my prev/next buttons will point to protocol next.

I don't mind waiting for his to be approved.

Copy link
Member

derberg commented Nov 8, 2022

@alequetzalli ok

/dnm

and you just merge whenever you want

@quetzalliwrites
Copy link
Member Author

well no, I can't merge this myself because I can't merge my own PRs 😂😜

I will still need you or someone to merge it when it's time to merge nelson's protocol doc

@derberg

@derberg
Copy link
Member

derberg commented Nov 9, 2022

@alequetzalli fair point, makes sense 🤣 just ping me when you need to merge it 😄

@quetzalliwrites
Copy link
Member Author

yo, you forgot to merge these PRs @derberg .. remember you are to merge the protocol and application docs together

Copy link
Member

@akshatnema akshatnema left a comment

Choose a reason for hiding this comment

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

LGTM @alequetzalli, I made a couple of changes in other files also to avoid creating a new PR and requesting for approvals. They are just made to align the UpNext and GoBack buttons correctly. Anything from your side @derberg?

@akshatnema
Copy link
Member

/rtm

@derberg
Copy link
Member

derberg commented Dec 6, 2022

folks, I'm changing settings for this PR so I can merge as admin. There is no other way, because for now except for readme @alequetzalli is the only codeowner for markdown files, together with the bot.

@derberg derberg merged commit 195b486 into asyncapi:master Dec 6, 2022
@quetzalliwrites
Copy link
Member Author

thanks for helping us merge these!!

@derberg

@quetzalliwrites
Copy link
Member Author

quetzalliwrites commented Dec 6, 2022

LGTM @alequetzalli, I made a couple of changes in other files also to avoid creating a new PR and requesting for approvals. They are just made to align the UpNext and GoBack buttons correctly. Anything from your side @derberg?

yo @akshatnema , you messed up my order here.. O_O I intentionally wanted the message doc to go right after the channel doc. But you assumed and changed the order of these pages, so now I have to go back in and fix it with a new PR.

There is a reason I teamed with Lukasz and had us wait to merge my application PR with the protocol one; we had already planned the buttons ahead of time.

The next time you want to make a change, don't assume. Please ask first via a commit suggestion.

@akshatnema
Copy link
Member

akshatnema commented Dec 7, 2022

But you assumed and changed the order of these pages, so now I have to go back in and fix it with a new PR.

@alequetzalli If you look into the changes made in the commit 36879a5 (#992) (made by me), it only comprises regarding the changes in the UpNext and GoBack buttons, which has no relation with the order of the pages shown in the documentation.

image

The order you see here is determined using the metadata of the Doc pages created inside the categories specified there. So, you probably have to change weights parameter for the pages, and make these buttons according to them. I don't know what's the correct order of the pages. What I saw yesterday in the PR is shown above and I made the buttons of the pages connected to each other using this order.

@derberg
Copy link
Member

derberg commented Dec 8, 2022

yeah, @alequetzalli I actually approved @akshatnema change as for me, it also looked weird that buttons do not match the nav order, so my brain assumed it must be wrong 🤷🏼 and because I promised to merge both at the same time, we did it the best we can.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/docs Specify what technical area given issue relates to. Its goal is to ease filtering good first issues. 📑 docs gsod This label should be used for issues or discussions related to ideas for Google Season of Docs ready-to-merge
Projects
Status: Done 🚀
Development

Successfully merging this pull request may close these issues.

[📑 Docs]: new application concept doc
5 participants