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

[CI] Add markdown check for normalized code-blocks + .md page fixes #3177

Merged

Conversation

chalin
Copy link
Contributor

@chalin chalin commented Aug 14, 2023

  • Contributes to CI improvements - 2023/08 #3122
  • This PR is best reviewed by ignoring whitespace changes
  • Adds an NPM script named fix:markdwon which will run markdownlint in "fix" mode
  • Adds a custom markdownlint rule, named trim-code-block-and-unindent, to check and optionally clean up fenced code blocks of:
    • Excessive blank lines at the start or end of the code
    • Unnecessary indentation
  • Adds trim-code-block-and-unindent:true and no-trailing-spaces:true to the markdownlint config
  • Fixes fenced code blocks in our docs using the new rule
  • Notes:
    • Custom rules are controlled in the same way as other rules. So they can be turned on/off for the project or selectively within a file.
    • The added custom rule is a bit hacky (I didn't quite understand how markdownlint rules worked when I started 🤷🏼‍♂️), but it works so I'm committing it now. I'll rework the rule later.

@chalin chalin added the CI/infra CI & infrastructure label Aug 14, 2023
@chalin chalin requested review from a team and dmitryax and removed request for a team August 14, 2023 21:00
Copy link
Contributor

@cartermp cartermp left a comment

Choose a reason for hiding this comment

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

Hmmm. What is "unecessary indent" here for code blocks? Maybe a sample would help demonstrate. I don't understand offhand how this accounts for whitespace-sensitive languages.

@chalin
Copy link
Contributor Author

chalin commented Aug 15, 2023

Maybe "excessive indentation" would be a better term than "unnecessary indentation"?

Here's an example from the PR:

Screenshot 2023-08-15 at 08 51 13

The algorithm will shift the code in the code block to the left as much as possible while preserving the relative indentation of non-whitespace characters. Of the 25+ files in this PR that were fixed, do you see any that were incorrectly adjusted (I've re-enabled view-whitespace diffs).

@chalin chalin force-pushed the chalin-im-normalize-code-blocks-2023-08-12 branch from c0caef5 to 83ffd24 Compare August 15, 2023 12:55
Copy link
Contributor

@cartermp cartermp left a comment

Choose a reason for hiding this comment

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

Okay, thanks, makes sense. So long as relative indentation is preserved then this is good.

@cartermp cartermp merged commit 69314d9 into open-telemetry:main Aug 15, 2023
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI/infra CI & infrastructure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants