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

Markdownlint fixes #30812

Merged
merged 2 commits into from
May 18, 2020
Merged

Markdownlint fixes #30812

merged 2 commits into from
May 18, 2020

Conversation

mdo
Copy link
Member

@mdo mdo commented May 14, 2020

Addresses some Markdown linting found from testing out markdownlint in #29919.

@XhmikosR
Copy link
Member

XhmikosR commented May 14, 2020

@mdo did you use my branch https://github.com/twbs/bootstrap/tree/master-xmr-markdownlint? Because there are false positives Markdownlint cannot handle hence why I haven't made a PR to actually add Markdownlint.

EDIT: here's the upstream issue DavidAnson/markdownlint#249

@mdo
Copy link
Member Author

mdo commented May 14, 2020

I didn't use your branch—I use markdownlint at GitHub and just ran the CLI against our docs to clear out the easy wins. There are tons of false positives due to our docs tags and what not.

README.md Outdated
[![](https://opencollective.com/bootstrap/sponsor/7/avatar.svg)](https://opencollective.com/bootstrap/sponsor/7/website)
[![](https://opencollective.com/bootstrap/sponsor/8/avatar.svg)](https://opencollective.com/bootstrap/sponsor/8/website)
[![](https://opencollective.com/bootstrap/sponsor/9/avatar.svg)](https://opencollective.com/bootstrap/sponsor/9/website)
[![OC sponsor](https://opencollective.com/bootstrap/sponsor/0/avatar.svg)](https://opencollective.com/bootstrap/sponsor/0/website)
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if "OC Sponsor" is better than nothing. @patrickhlauke

Copy link
Member

Choose a reason for hiding this comment

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

so i take it there's no way for us to somehow get that pulled in via third party or something and have it provided with more accurate alt etc? (like a validation badge or similar that actually gets generated separately and dynamically pulled in)

if not, then...yeah, better than nothing i guess

Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately, I'm not aware of a way. But perhaps we should use OC sponsor 1 etc?

Copy link
Member

Choose a reason for hiding this comment

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

i guess you could. that would still not provide the equivalent of what sponsor it is, but i guess it would at least indicate it's not always the same image. least worst fix, probably

@XhmikosR
Copy link
Member

There are tons of false positives due to our docs tags and what not.

I have disabled some rules in my branch https://github.com/twbs/bootstrap/runs/589127421#step:9:235

@mdo mdo force-pushed the markdown-linting branch from f48c3f2 to b5e502e Compare May 14, 2020 05:34
@mdo
Copy link
Member Author

mdo commented May 14, 2020

To clarify, this isn't meant to add a script or Action anywhere—it's just cleaning up some Markdown :). I'll defer to another PR for implementing it and running it as a check for us everywhere.

@XhmikosR
Copy link
Member

To clarify, this isn't meant to add a script or Action anywhere

I know, just trying to get the best of the two branches :)

So, you prefer a - for ul style?

That being said, let's wait to hear about the alt change from @patrickhlauke. Personally I just ignored these specific errors in README.md in my branch and allowed them everywhere else.

@XhmikosR XhmikosR changed the title Markdown linting: Use dashes for bulleted lists Markdownlint fixes May 14, 2020
@XhmikosR
Copy link
Member

@patrickhlauke friendly ping :)

mdo and others added 2 commits May 18, 2020 16:18
- Always use dashes for bullets
- Add some basic alt text for our readme OC sponsors
@XhmikosR XhmikosR force-pushed the markdown-linting branch from af3734d to 849e018 Compare May 18, 2020 13:19
@XhmikosR XhmikosR merged commit 62626db into master May 18, 2020
@XhmikosR XhmikosR deleted the markdown-linting branch May 18, 2020 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants