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

Add documentation style guide and tool configs #5320

Merged
merged 6 commits into from
Jan 18, 2025

Conversation

danbarr
Copy link
Contributor

@danbarr danbarr commented Jan 17, 2025

Summary

This PR proposes a documentation style guide and configurations for Prettier, markdownlint, and ESLint tooling. The style guide is a subset of Google's documentation and Markdown style guides.

If accepted, I will follow up with another PR to format/lint all of the current docs files.

A final step will be to propose automated checks by way of GitHub workflows to validate formatting on PRs that modify docs.

Change Type

Mark the type of change your PR introduces:

  • Bug fix (resolves an issue without affecting existing features)
  • Feature (adds new functionality without breaking changes)
  • Breaking change (may impact existing functionalities or require documentation updates)
  • Documentation (updates or additions to documentation)
  • Refactoring or test improvements (no bug fixes or new functionality)

Testing

Tested that the prettier, markdownlint, and eslint configurations are reporting issues as expected. Auto-generated docs (API/CLI/protocol) have been excluded/ignored.

Review Checklist:

  • Reviewed my own code for quality and clarity.
  • Added comments to complex or tricky code sections.
  • Updated any affected documentation.
  • (N/A) Included tests that validate the fix or feature.
  • (N/A) Checked that related changes are merged.

@danbarr danbarr requested a review from a team as a code owner January 17, 2025 20:48
@coveralls
Copy link

coveralls commented Jan 17, 2025

Coverage Status

coverage: 56.635% (-0.003%) from 56.638%
when pulling 4253214 on danbarr:docs/add-style-configs
into 19c31fe on mindersec:main.

.gitattributes Show resolved Hide resolved
"tables": false
},
"no-bare-urls": false,
"no-duplicate-heading": false,
Copy link
Member

Choose a reason for hiding this comment

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

Why is this false?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure which one you mean, but

  • tables for line-length: wrapping tables makes them really, really hard to edit
  • no-bare-urls: the auto-fix for this (wrapping a bare URL in <>'s) breaks Docusaurus rendering since it runs everything thru an MDX parser, even .md files. I'd like to be able to check for these but not auto-fix them, but currently markdownlint doesn't support making that distinction.
  • no-duplicate-heading: I think we can actually turn this one back on, the rule reference pages have a lot of these but I put this into a more-specific config file just in that folder so I don't think any of this rule gets flagged elsewhere. I'll do that on the next PR.

"style": "leading_and_trailing"
},
"ul-style": {
"style": "dash"
Copy link
Member

Choose a reason for hiding this comment

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

I always use *, but this dog can learn new tricks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used to also; the rationale I drew this from was that it's easier to read when you do things like this:

* **A bold bullet**

vs.

- **A bold bullet**

But frankly, this is a nit - if * are more comfortable/natural I'm not opposed to changing this - consistency is more important than which character.

Markdownlint also support a sublist style which uses different symbols as you nest:

* Item 1
  + Item 2
    - Item 3
  + Item 4
* Item 4
  + Item 5


```bash
npm run start
Copy link
Member

Choose a reason for hiding this comment

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

This needs to be in the docs repo, right? Maybe that's obvious, but the Makefile is at the top level.

Suggested change
npm run start
cd docs
npm run start

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops good catch, I added this in and missed that the cd docs is below it now. I'll fix in the next PR.

docs/STYLE-GUIDE.md Show resolved Hide resolved
@evankanderson evankanderson merged commit 45e84c4 into mindersec:main Jan 18, 2025
28 checks passed
@danbarr danbarr deleted the docs/add-style-configs branch January 18, 2025 05:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants