Skip to content
This repository was archived by the owner on Jan 8, 2025. It is now read-only.

Change from default to one: [lint-ordered-list-marker-value one]#341

Merged
lsgunn-teleport merged 2 commits intomainfrom
LG/change-ordered-list-to-one
Jul 25, 2023
Merged

Change from default to one: [lint-ordered-list-marker-value one]#341
lsgunn-teleport merged 2 commits intomainfrom
LG/change-ordered-list-to-one

Conversation

@lsgunn-teleport
Copy link
Copy Markdown
Contributor

@lsgunn-teleport lsgunn-teleport commented Jul 13, 2023

The linter was throwing warnings for numbered steps like this:
116:1-116:69 warning Marker should be 3, was 1
118:1-120:84 warning Marker should be 4, was 1

That's because the docs engine uses the lint-ordered-list-marker-value plugin (GitHub) to enforce numbering, and the default is ordered (https://www.npmjs.com/package/remark-lint-ordered-list-marker-value#api).

However, the standard process is to use "1." in Markdown files for auto-numbering. This PR changes this line:
["lint-ordered-list-marker-value", "one"]

Change to single per the recommendation in the API doc:
https://www.npmjs.com/package/remark-lint-ordered-list-marker-value#api

Ran yarn markdown-lint.
⚠ 656 warnings
Changes made in 14.x are in these PRs:
gravitational/teleport#29143
gravitational/teleport#29094
gravitational/teleport#29093
gravitational/teleport#29086

@vercel
Copy link
Copy Markdown

vercel Bot commented Jul 13, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 13, 2023 8:52pm

@ptgott
Copy link
Copy Markdown
Contributor

ptgott commented Jul 13, 2023

This looks good, though we should hold off on merging until we fix the linter violations in gravitational/teleport. According to yarn markdown-lint there are 655 warnings across all docs versions.

@alexfornuto
Copy link
Copy Markdown
Contributor

Resolves #164

@lsgunn-teleport
Copy link
Copy Markdown
Contributor Author

There's a bit of a chicken/egg problem. None of the PRs to fix the issues can pass the current linter when modified until this PR is merged. I think I have most of the fixes queued up to follow this in cascading fashion (with backports to the older versions of files.
@alexfornuto, what do you think?

@alexfornuto
Copy link
Copy Markdown
Contributor

@alexfornuto, what do you think?

IIRC the ooo is to merge the linter fix first, then re-run the tests on these PRs to merge them. But come to think of it, since no one PR has all the changes, none of them will pass (assuming the linter will at that point only allow 1-type lists). In that case, I'd pick one of these PRs and merge the rest into it, then once the docs PR is merged, re-run the tests and merge on that mega-pr.

@lsgunn-teleport
Copy link
Copy Markdown
Contributor Author

okay, that's a good suggestion. The batch PRs are consolidated. Now, if you approve, I'll merge this change then get the other updates in a follow-on.

@lsgunn-teleport
Copy link
Copy Markdown
Contributor Author

lsgunn-teleport commented Jul 18, 2023

I think all of the fixes are in and conflicts are resolved 🤞. Running yarn markdown-lint on the branch with the linter change shows no issues found.
content/14.x/docs/pages/server-access/rbac.mdx: no issues found
content/14.x/docs/pages/upcoming-releases.mdx: no issues found
✨ Done in 23.63s.
@alexfornuto, are we ready to pull the trigger on the linter change?

@ptgott
Copy link
Copy Markdown
Contributor

ptgott commented Jul 24, 2023

@lsgunn-teleport Once we backport the content changes, we'll be good to go to merge!

@alexfornuto
Copy link
Copy Markdown
Contributor

@ptgott all the backports are pre-created and approved. If everyone's confident, I think it's time to :shipit:

@ptgott
Copy link
Copy Markdown
Contributor

ptgott commented Jul 25, 2023

Still getting a lot of "Marker should be" warnings after switching to this branch and running yarn git-update:

$ yarn markdown-lint |& grep "Marker should be" | wc -l
     612

@lsgunn-teleport
Copy link
Copy Markdown
Contributor Author

lsgunn-teleport commented Jul 25, 2023

@ptgott @alexfornuto
To properly test this, switch to the following branches:
LG/backport-29143-branch/v11
LG/backport-29143-branch/v12
afornuto/backport-29143-branch/v13
LG/lint-numbering-4-14x
LG/change-ordered-list-to-one

After these branches are your current environment, you shouldn't see any warnings.

@lsgunn-teleport lsgunn-teleport merged commit 4c945e7 into main Jul 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants