-
Notifications
You must be signed in to change notification settings - Fork 22.4k
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(fix): Apply See also guidelines as examples #25380
Conversation
Preview URLs
External URLs (4)URL:
URL:
(comment last updated: 2023-03-20 18:55:07) |
- [`async function` expression](/en-US/docs/Web/JavaScript/Reference/Operators/async_function) | ||
- {{jsxref("AsyncFunction")}} object | ||
- [Top level await](https://v8.dev/features/top-level-await) on v8.dev | ||
- {{jsxref("Statements/async_function", "async function")}}: Declaration that defines where the `await` keyword is permitted within the `async function` body |
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.
Is this description necessary per the guideline? Won't "async function
declaration" work?
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.
As per the guideline, this description is completely optional - so not at all necessary. And we can leave the list items how they were in this case. I just thought by adding a description, the three similar sounding terms could perhaps be differentiated to give readers the reason to click on the appropriate link.
By the way, as per #25191 (comment), we're not adding that descriptive word after the keyword any more. So this would just be async function
.
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 would be perfectly fine with async function
itself if there aren't two things with the same title (async function
expression & async function
declaration). The purpose of that word is just to disambiguate. However, I feel like the description is unnecessary.
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.
Forgot to ask. So no description for all the three items?
Luckily, "async function
expression" has "expression" in the page title. So the following two bullets work:
async function
async function
expression
But for AsyncFunction
, we'd need to add the word "object" (as is already there), which we're saying in the guidelines not to do. The other option is to add a description for this one. What do you think about this one?
I feel this can be justified better (minimal description where possible) than adding the word "object".
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.
Just saying "AsyncFunction
" without the "object" is fine. We do that in other places. "async function
expression" is a bit sad because we can no longer use macros, but otherwise, this looks much better 👍
If the suggested changes to the "See also" sections look okay here, I would appreciate to get an approval and land this, so that we can also land the guidelines PR (#25191). |
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.
JS change LGTM 👍
Landing this to unblock you @dipikabh - it all looks great to me 👍 |
Thanks, @Rumyra! |
Description
This is a follow up to "See also" guidelines pull request #25191.
This pull request applies the guidelines to a few existing "See also" sections, as were discussed as examples in the discussion. One of the examples is already covered in the previous pull request: /en-us/web/css/@layer/index.md
As a result of the feedback in #25191, there are slight differences from what was originally proposed and discussed. The guidelines have been updated in #25191.
In general, the following structure is being followed:
<link text>: Description phrase(no punctuation)
The description phrase is optional. The list item should begin with the link text. Keeping all link texts in a list at the beginning will help to easily and quickly scan the list.