Skip to content

fix: Bad code fence indenting in ordered lists - #675

Merged
martin389 merged 1 commit into
github:mainfrom
nschonni:code-fence-indents
Nov 24, 2020
Merged

fix: Bad code fence indenting in ordered lists#675
martin389 merged 1 commit into
github:mainfrom
nschonni:code-fence-indents

Conversation

@nschonni

@nschonninschonni commented Oct 20, 2020

Copy link
Copy Markdown
Contributor

Why:

Split out commit from #541 for markdown sections that cause parser issues with markdownlint because of incorrect indenting of start/end code fences

What's being changed:

  • Un-started table row. Caused Crowdin to mangle it in the translated copies
  • Code fences inside of ordered list that didn't have at lease 3 spaces and some that had different open/closing indents

Check off the following:

@janiceilene

janiceilene commented Oct 20, 2020

Copy link
Copy Markdown
Contributor

Thanks for opening a PR @nschonni! I'll get this triaged for review 🌟

Edit: I'll wait to triage this one until we get a chance to look at the linked PR 💛

@nschonninschonni mentioned this pull request Oct 21, 2020
4 tasks
@nschonni

Copy link
Copy Markdown
ContributorAuthor

@janiceilene I've removed the content changes from the other PR now, since the original issue requiring them has been fixed in Markdownlint. They are still content issues, so I'll leave the for review here

@janiceilenejaniceilene added the hacktoberfest-accepted We might not merge this PR before Nov 1st, but it's a wonderful Hacktoberfest contribution! label Oct 27, 2020
@github-actions

Copy link
Copy Markdown
Contributor

This PR is stale because it has been open 7 days with no activity and will be automatically closed in 3 days. To keep this PR open, update the PR by adding a comment or pushing a commit.

@github-actionsgithub-actionsBot added the stale There is no recent activity on this issue or pull request label Nov 4, 2020
@nschonni
nschonniforce-pushed the code-fence-indents branch 2 times, most recently from 4351cf6 to 1fdcec6CompareNovember 4, 2020 18:39

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

This is the only functional change

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Looks like this got Crowdin to parse correctly again from the changes in #1603

@nschonni

Copy link
Copy Markdown
ContributorAuthor

@janiceilene@rachmari looks like the stale bot doesn't want to dismiss the label after pushing/rebasing

@rachmarirachmari removed the stale There is no recent activity on this issue or pull request label Nov 4, 2020
@nschonni
nschonniforce-pushed the code-fence-indents branch 3 times, most recently from 71c6c87 to bf19b22CompareNovember 6, 2020 22:56
@janiceilene

Copy link
Copy Markdown
Contributor

Looking at this again briefly, this can be reviewed by any writer! I removed @rachmari and I as reviewers and I'll move this to the anyone column for triage.

@janiceilenejaniceilene added the content This issue or pull request belongs to the Docs Content team label Nov 9, 2020

@martin389martin389 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for this PR @nschonni! I've added a couple of suggestions as comments.

@nschonni
nschonniforce-pushed the code-fence-indents branch 3 times, most recently from 3f279e6 to 33c2fccCompareNovember 19, 2020 00:16
@janiceilene

Copy link
Copy Markdown
Contributor

👋 @martin389 It looks like this one's ready for your 👀 again

@martin389

martin389 commented Nov 20, 2020

Copy link
Copy Markdown
Contributor

Hi @nschonni 👋

I had a look at Configuring high availability replication for a cluster, and some of the rendering might need to be checked:

  • In step 12 of Adding passive nodes to the cluster configuration file, the contents of the warning admonition seem to have escaped and are rendering underneath the block:
    image
    This has also reset the procedure's numbering back to 1, when it should be continuing to step 13 from there.

@martin389martin389 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for these changes, @nschonni! 🎉

I've finished my review, and added a comment above for one of the sections:

The rest looks good to me: 🚀

@nschonni

Copy link
Copy Markdown
ContributorAuthor

🤦 sorry about the repeat reviews. I rebased and checked that file on dev side-by-side again with poduction and they're looking the same.
I notice that there appears to be something funny going on with the list numbering with the warning/note blocks resetting the numbering, but it appears in production as well. It looks like the correct <ol start='#'> is being correctly set, but the styling is going back to 1 🤷

@martin389

martin389 commented Nov 23, 2020

Copy link
Copy Markdown
Contributor

@nschonni

🤦 sorry about the repeat reviews. I rebased and checked that file on dev side-by-side again with poduction and they're looking the same.
I notice that there appears to be something funny going on with the list numbering with the warning/note blocks resetting the numbering, but it appears in production as well. It looks like the correct <ol start='#'> is being correctly set, but the styling is going back to 1 🤷

No worries, thanks so much for checking on this 🚀
It does seem like it should all be rendering as expected, so I'll do some digging around here too.

@martin389

Copy link
Copy Markdown
Contributor

👋 For docs tooling folks:

The procedure numbering resets a few times in this staging article: "Configuring high availability replication for a cluster". Specific examples are listed below:

  • "Assigning active nodes to the primary datacenter": (stage)(prod) - Step 5 in prod becomes step 1 in stage."
  • "Adding passive nodes to the cluster configuration file": (stage)(prod) - Step 2 in prod becomes step 1 in stage. Numbering also resets again a few more times throughout this section.

@nschonni

Copy link
Copy Markdown
ContributorAuthor

Sounds like it might have been an internal issue that was fixed #1552 (comment)
I'll try rebasing now to see if it fixes it

@nschonni

Copy link
Copy Markdown
ContributorAuthor

@martin389 rebase looks like it fixed it 😄

@martin389martin389 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@martin389
martin389 merged commit 93c24ac into github:mainNov 24, 2020
@github-actions

Copy link
Copy Markdown
Contributor

Thanks very much for contributing! Your pull request has been merged 🎉 You should see your changes appear on the site in approximately 24 hours.

@martin389

Copy link
Copy Markdown
Contributor

Thanks again for these fixes, @nschonni! 🎉

@nschonni
nschonni deleted the code-fence-indents branch November 24, 2020 02:41
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contentThis issue or pull request belongs to the Docs Content teamhacktoberfest-acceptedWe might not merge this PR before Nov 1st, but it's a wonderful Hacktoberfest contribution!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@nschonni@janiceilene@martin389@rachmari