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

Fix definition lists #19863

Merged
merged 3 commits into from
Aug 23, 2022
Merged

Fix definition lists #19863

merged 3 commits into from
Aug 23, 2022

Conversation

OnkarRuikar
Copy link
Contributor

These are the remaining cases in the repo.


Note: Also deliberately added some md errors to test

@OnkarRuikar OnkarRuikar requested a review from a team as a code owner August 23, 2022 07:14
@OnkarRuikar OnkarRuikar requested a review from a team August 23, 2022 07:14
@OnkarRuikar OnkarRuikar requested review from a team as code owners August 23, 2022 07:14
@OnkarRuikar OnkarRuikar requested review from rebloor, teoli2003 and estelle and removed request for a team August 23, 2022 07:14
@github-actions github-actions bot added Content:CSS Cascading Style Sheets docs Content:HTTP HTTP docs Content:Other Any docs not covered by another "Content:" label Content:SVG SVG docs Content:WebExt WebExtensions docs labels Aug 23, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Aug 23, 2022

Preview URLs

Flaws

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

URL: /en-US/docs/Web/CSS/-webkit-mask-position-x
Title: -webkit-mask-position-x
on GitHub
Flaw count: 2

  • macros:
    • /en-US/docs/Web/CSS/-webkit-mask-position redirects to /en-US/docs/Web/CSS/mask-position
    • /en-US/docs/Web/CSS/-webkit-mask-origin redirects to /en-US/docs/Web/CSS/mask-origin

URL: /en-US/docs/Web/CSS/-webkit-mask-position-y
Title: -webkit-mask-position-y
on GitHub
Flaw count: 2

  • macros:
    • /en-US/docs/Web/CSS/-webkit-mask-position redirects to /en-US/docs/Web/CSS/mask-position
    • /en-US/docs/Web/CSS/-webkit-mask-origin redirects to /en-US/docs/Web/CSS/mask-origin

External URLs

URL: /en-US/docs/Web/SVG/Tutorial/Texts
Title: Texts
on GitHub

No new external URLs


URL: /en-US/docs/Web/HTTP/Headers/Authorization
Title: Authorization
on GitHub

No new external URLs


URL: /en-US/docs/Web/CSS/time
Title: <time>
on GitHub

No new external URLs


URL: /en-US/docs/Web/CSS/-webkit-mask-position-x
Title: -webkit-mask-position-x
on GitHub

No new external URLs


URL: /en-US/docs/Web/CSS/-webkit-mask-position-y
Title: -webkit-mask-position-y
on GitHub

No new external URLs


URL: /en-US/docs/Mozilla/Add-ons/WebExtensions/API/runtime/getBrowserInfo
Title: runtime.getBrowserInfo()
on GitHub

No new external URLs

(this comment was updated 2022-08-23 07:45:17.800553)

Comment on lines 31 to 34
##### Dummy Test Section

Testing the Github md lint action.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why should this lead to an MD error (level 5?)?

Copy link
Contributor Author

@OnkarRuikar OnkarRuikar Aug 23, 2022

Choose a reason for hiding this comment

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

yeah! [ref]

MD001 - Heading levels should only increment by one level at a time

But it also didn't complain about the trailing spaces on line 33. 😕
Why did the lint succeed? https://github.com/mdn/content/runs/7967730361?check_suite_focus=true

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does it check the branch or the main?

I've committed more spaces. It is working but looks like the linter isn't complaining about two trailing spaces. For one trailing space it did give the annotation.

We've disabled the MD001 in the config file so there is no error for that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Note that we don't want this, so it is ok for us to remove them.

@teoli2003
Copy link
Contributor

So it looks like we have been defensive on which Markdownlint rule to activate. Good! (We can tighten the rules little by little).

Let's fix the errors and see how people react to this test.

@teoli2003 teoli2003 merged commit c8b05dd into mdn:main Aug 23, 2022
@OnkarRuikar
Copy link
Contributor Author

Can we make the annotations orange instead of red?
Like this:
orrange

We do not want to force the contributors to fix these errors.

@teoli2003
Copy link
Contributor

Yes, can you open a PR so I can merge it?

@OnkarRuikar
Copy link
Contributor Author

Yes, can you open a PR so I can merge it?

This has been completed in Make lint annotations warnings.

@@ -54,6 +54,8 @@ let gettingInfo = browser.runtime.getBrowserInfo();
gettingInfo.then(gotBrowserInfo);
```

{{WebExtExamples}}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this best left in, it would automatically pick up any examples in the web extensions examples repository that use the API

@OnkarRuikar OnkarRuikar deleted the fix_def_list branch July 31, 2024 02:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:CSS Cascading Style Sheets docs Content:HTTP HTTP docs Content:Other Any docs not covered by another "Content:" label Content:SVG SVG docs Content:WebExt WebExtensions docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants