-
Notifications
You must be signed in to change notification settings - Fork 24
feat(Fift): Assembler page #1542
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
base: main
Are you sure you want to change the base?
Conversation
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.
Thanks for the updates here; I’ve left a couple of suggestions in languages/fift/fift-assembler.mdx: please apply the inline suggestions.
| --- | ||
| title: "Assembler" | ||
| sidebarTitle: "Assembler" | ||
| noindex: "true" | ||
| --- |
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.
[HIGH] Frontmatter title is an unsupported one‑word generic label
The new page frontmatter sets title: "Assembler" and sidebarTitle: "Assembler", which is a one‑word generic title that does not clearly describe the page content. The style guide’s “Title vs sidebar semantics” rule requires page titles to be context‑free and self‑describing, and explicitly forbids one‑word generic titles such as “Overview” or “Introduction” on non‑top‑level pages. This page lives under the Fift group rather than at a top‑level route, and “Assembler” is not a proper name or established term here, so the current title reduces discoverability and makes deep links and search results less informative. The sidebarTitle can remain concise and contextual, but the title should be expanded to clearly identify the Fift assembler topic.
| --- | |
| title: "Assembler" | |
| sidebarTitle: "Assembler" | |
| noindex: "true" | |
| --- | |
| --- | |
| title: "Fift assembler" | |
| sidebarTitle: "Assembler" | |
| noindex: "true" | |
| --- |
Please leave a reaction 👍/👎 to this suggestion to improve future reviews for everyone!
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.
Done
| --- | ||
| title: "Assembler" | ||
| sidebarTitle: "Assembler" | ||
| noindex: "true" |
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.
[MEDIUM] Missing noindex boolean type in frontmatter
The frontmatter noindex key is set to the string "true" instead of a boolean literal. Site tooling and search/indexing logic typically expect frontmatter flags like noindex to be consistently typed booleans so they can be parsed without special cases. Leaving this as a string can cause incorrect indexing behavior or require bespoke parsing logic.
| noindex: "true" | |
| noindex: true |
Please leave a reaction 👍/👎 to this suggestion to improve future reviews for everyone!
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 really an issue?
Closes #1540.