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.

Sign up for GitHub

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

Jump to bottom

Lint front-matter using the work in progress fmlinter #26174

Merged
merged 2 commits into from
Apr 13, 2023
Merged

Lint front-matter using the work in progress fmlinter #26174

merged 2 commits into from
Apr 13, 2023

Conversation

OnkarRuikar
Copy link
Contributor

These are cherry picked issues. There is more to come.

I've tested the rendered changes yari has no issues with it.

@OnkarRuikar OnkarRuikar requested review from a team as code owners April 13, 2023 05:43
@OnkarRuikar OnkarRuikar requested a review from a team April 13, 2023 05:43
@OnkarRuikar OnkarRuikar requested review from a team as code owners April 13, 2023 05:43
@OnkarRuikar OnkarRuikar requested review from fred-wang, jpmedley, Ryuno-Ki, sideshowbarker, teoli2003, schalkneethling and estelle and removed request for a team April 13, 2023 05:43
@github-actions github-actions bot added Content:CSS Cascading Style Sheets docs Content:Glossary Glossary entries Content:HTML Hypertext Markup Language docs Content:HTTP HTTP docs Content:JS JavaScript docs labels Apr 13, 2023
@github-actions github-actions bot added Content:MathML MathML docs Content:Other Any docs not covered by another "Content:" label Content:SVG SVG docs Content:WebAPI Web API docs Content:WebExt WebExtensions docs labels Apr 13, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Apr 13, 2023 •
edited
Loading

Preview URLs (168 pages)
Flaws (21)

Note! 156 documents with no flaws that don't need to be listed. 🎉

URL: /en-US/docs/Web/HTML/Element/template
Title: <template>: The Content Template element
Flaw count: 1

  • macros:
    • /en-US/docs/Web/HTML/Element/shadow does not exist

URL: /en-US/docs/Web/HTML/Element/script
Title: <script>: The Script element
Flaw count: 1

  • macros:
    • /en-US/docs/Web/API/Element/load_event does not exist

URL: /en-US/docs/Web/HTML/Element/portal
Title: <portal>: The Portal element
Flaw count: 1

  • macros:
    • /en-US/docs/Web/API/HTMLPortalElement does not exist

URL: /en-US/docs/Web/HTML/Element/bdi
Title: <bdi>: The Bidirectional Isolate element
Flaw count: 3

  • macros:
    • The fourth to sixth parameters of 'EmbedLiveSample' are deprecated
    • The fourth to sixth parameters of 'EmbedLiveSample' are deprecated
    • The fourth to sixth parameters of 'EmbedLiveSample' are deprecated

URL: /en-US/docs/Web/HTML/Element/applet
Title: <applet>: The Embed Java Applet element
Flaw count: 1

  • macros:
    • /en-US/docs/Web/API/HTMLAppletElement does not exist

URL: /en-US/docs/Web/HTML/Element/dir
Title: <dir>: The Directory element
Flaw count: 1

  • macros:
    • /en-US/docs/Web/API/HTMLDirectoryElement does not exist

URL: /en-US/docs/Web/HTML/Element/track
Title: <track>: The Embed Text Track element
Flaw count: 1

  • macros:
    • /en-US/docs/Web/API/HTMLTrackElement/track does not exist

URL: /en-US/docs/Web/HTML/Attributes/size
Title: HTML attribute: size
Flaw count: 1

  • bad_bcd_queries:
    • No BCD data for query: html.elements.attribute.size

URL: /en-US/docs/Web/HTML/Attributes/required
Title: HTML attribute: required
Flaw count: 1

  • bad_bcd_queries:
    • No BCD data for query: html.elements.attributes.required

URL: /en-US/docs/Web/HTML/Attributes/multiple
Title: HTML attribute: multiple
Flaw count: 2

  • broken_links:
    • Can't resolve /en-US/docs/Web/HTML/Attributes/list
    • Can't resolve /en-US/docs/Web/API/XMLHttpRequest/multipart

URL: /en-US/docs/Web/SVG/Attribute/crossorigin
Title: SVG attribute: crossorigin
Flaw count: 1

  • bad_bcd_queries:
    • No BCD data for query: api.SVGImageElement.crossOrigin

URL: /en-US/docs/Web/CSS/CSS_Basic_User_Interface
Title: CSS Basic User Interface
Flaw count: 7

  • macros:
    • /en-US/docs/Web/CSS/caret does not exist
    • /en-US/docs/Web/CSS/caret-shape does not exist
    • /en-US/docs/Web/CSS/nav-down does not exist
    • /en-US/docs/Web/CSS/nav-left does not exist
    • /en-US/docs/Web/CSS/nav-right does not exist
    • and 1 more flaws omitted
  • broken_links:
    • Can't resolve /en-US/docs/Web/CSS/CSS_Basic_User_Interface/Using_URL_values_for_the_cursor_property
External URLs (1)

URL: /en-US/docs/Glossary/Replay_attack
Title: Replay attack

(comment last updated: 2023-04-13 05:53:49)

teoli2003
teoli2003 approved these changes Apr 13, 2023
View reviewed changes
Copy link
Contributor

@teoli2003 teoli2003 left a comment

Choose a reason for hiding this comment

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

👍

@teoli2003 teoli2003 merged commit 942a529 into mdn:main Apr 13, 2023
@OnkarRuikar OnkarRuikar deleted the fix_frontmatter branch April 13, 2023 06:15
@OnkarRuikar OnkarRuikar mentioned this pull request Apr 13, 2023
Merged
Mehrm0 pushed a commit to Mehrm0/content that referenced this pull request Apr 14, 2023
@OnkarRuikar @Mehrm0
Lint front-matter using the work in progress fmlinter (mdn#26174)
cd51d1b 
* Lint front-matter using the work in progress fmlinter

* Fix lint errors caused by husky
@OnkarRuikar OnkarRuikar mentioned this pull request Apr 28, 2023
Merged
Josh-Cena
Josh-Cena reviewed May 2, 2023
View reviewed changes
files/en-us/web/javascript/reference/errors/bad_regexp_flag/index.md
@@ -1,5 +1,5 @@
---
title: 'SyntaxError: invalid regular expression flag "x"'
title: "SyntaxError: invalid regular expression flag \"x\""
Copy link
Member

Choose a reason for hiding this comment

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

Hi @OnkarRuikar I've just noticed that this change is not compatible with Prettier, which optimizes the number of escaped quotes. Just FYI—we will eventually need to accommodate Prettier style.

Copy link
Contributor Author

@OnkarRuikar OnkarRuikar May 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Hi @OnkarRuikar I've just noticed that this change is not compatible with Prettier, which optimizes the number of escaped quotes. Just FYI—we will eventually need to accommodate Prettier style.

There is no point in using two tools for indenting the front-matter: there will be multiple mismatches like this in future.

Our validator can reorder the attributes, which the Prettier can’t. When our validator dumps(serialises) the corrected output it is going to indent the YAML by js-yaml’s rules, and they are not as flexible; we’ll have to use Prettier in the tool to reindent the YAML :(
We have 3 options:
a) Disable front-matter linting in Prettier. Can we disable the front-matter part in Prettier using some config option(not magic comments)?
b) Use Prettier in our validator to reindent the front-matter before writing to file.
c) If possible, write a Prettier plugin that will do the validations and formatting of front-matter that we want.
d) do nothing, as husky hook for our validator will auto format files before commit. Prettier will keep nagging though.

Also, in Prettier can we have infinite line length for YAML and 80 for rest of the content? Because there is going to be the same issue as discussed/voted here.

Option b) is dirty but the quickest of all. I am not sure if a) and c) are even possible.

cc/ @teoli2003, @wbamberg

Copy link
Member

Choose a reason for hiding this comment

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

I don't know how the validator is implemented, but I'm not suggesting anything to be done with Prettier. If possible, I wish the validator respects Prettier style in terms of quotes, wrapping, and indenting—no point in letting the validator do formatting. (In the same spirit as ESLint should not do formatting.) It should only care about key ordering, invalid values, etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Josh-Cena Josh-Cena Josh-Cena left review comments

@teoli2003 teoli2003 teoli2003 approved these changes

@fred-wang fred-wang Awaiting requested review from fred-wang fred-wang is a code owner automatically assigned from mdn/content-mathml

@jpmedley jpmedley Awaiting requested review from jpmedley jpmedley is a code owner automatically assigned from mdn/yari-content-mozilla-add-ons

@Ryuno-Ki Ryuno-Ki Awaiting requested review from Ryuno-Ki

@sideshowbarker sideshowbarker Awaiting requested review from sideshowbarker sideshowbarker is a code owner automatically assigned from mdn/yari-content-web-api

@schalkneethling schalkneethling Awaiting requested review from schalkneethling schalkneethling is a code owner automatically assigned from mdn/yari-content-html

@estelle estelle Awaiting requested review from estelle estelle is a code owner automatically assigned from mdn/yari-content-css

Assignees
No one assigned
Labels
Content:CSS Cascading Style Sheets docs Content:Glossary Glossary entries Content:HTML Hypertext Markup Language docs Content:HTTP HTTP docs Content:JS JavaScript docs Content:MathML MathML docs Content:Other Any docs not covered by another "Content:" label Content:SVG SVG docs Content:WebAPI Web API docs Content:WebExt WebExtensions docs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@OnkarRuikar @teoli2003 @Josh-Cena