-
-
Notifications
You must be signed in to change notification settings - Fork 59
feat: add markdownlint #538
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
base: master
Are you sure you want to change the base?
Conversation
it's "green" because of |
markdownlint does now run without |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! I am just worried by the configuration file. I have no experience with this tool, but on the check pages I see "aliases" which makes me think that maybe we can have something easier to digest (example line_length rather than MD013)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll be honest, in general I don't really see the point of markdownlint, especially when its recommendations make the source doc harder to read -- the express purpose of markdown in the first place. Or when it requests pointless changes like insisting on _underscore emphasis_
instead of *asterisks*
or requiring shell
instead of bash
. All that does is increase friction of getting people to write docs and posts in the first place.
I didn't do a thorough proofread, just highlighted instances of unhelpful changes.
@@ -65,14 +64,12 @@ that require the redundancy of multiple compilers. | |||
|
|||
{write a guide on how to deploy, configure, and use this architecture} | |||
|
|||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These extra spaces between headlines are incredibly helpful when reading the source doc. Please disable this check.
@@ -34,15 +34,15 @@ HDM --- OpenVoxServer | |||
Puppetboard --- OpenVoxDB | |||
OpenVoxServer --- Agent1("Agent 1") & Agent2("Agent 2") & Agent_n("Agent n") | |||
|
|||
click Foreman "https://www.theforeman.org/" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this code is a mermaid diagram
### OpenVox Stack | ||
|
||
We recommend managing each of these components with the supported module. | ||
|
||
* OpenVoxDB | ||
* [puppetlabs/puppetdb](https://forge.puppet.com/puppetlabs/puppetdb) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't hurt to have deeper indentation steps and it often makes it easier to read.
@@ -5,26 +5,26 @@ summary: A description of how to run the Vox Pupuli test suite for Puppet module | |||
--- | |||
|
|||
- [Running the tests in a local ruby environment](#running-the-tests-in-a-local-ruby-environment) | |||
* [Installing dependencies](#installing-dependencies) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's entirely valid and useful to use different markers to indicate list depth.
@@ -88,9 +88,9 @@ If you don't know if you need to install or update gems, you can just add `bundl | |||
Check out the following page if you want to add a test suite to your module or want | |||
to learn more about the Vox Pupuli test helpers: | |||
|
|||
* [voxpupuli-test](https://github.com/voxpupuli/voxpupuli-test) for unit testing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the intent of this list is perfectly clear, even if another part of the doc used a different marker
@@ -74,7 +74,7 @@ file { '/etc/puppetlabs/client-tools/puppetdb.conf': | |||
} | |||
``` | |||
|
|||
## Contribute?! | |||
## Contribute? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a stylistic choice and it's perfectly valid to express excitement in this way.
@@ -19,21 +19,21 @@ This can be done from a fork. | |||
|
|||
Now you can install the changelog generator: | |||
|
|||
```bash | |||
```shell |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this change is pointless
*Please note that in order to execute this rake task you must be in the __Collaborators__ group on GitHub for the module in question.* | ||
|
||
*Please also note that the task requires a configured gpg or ssh key in your local git settings to sign the git tag* | ||
* _Please note that in order to execute this rake task you must be in the __Collaborators__ group on GitHub for the module in question._ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this changes the rendering of the doc for no reason.
@@ -10,7 +10,7 @@ Well, *[[checks watch]]* would ya look at that. It's March 23, UTC, and that mea | |||
|
|||
[Martin](https://github.com/tuxmea) was the first to submit a nomination, and as promised Puppet will be sending him a brand spankin' fresh hoodie as soon as we've got the new design finalized. | |||
|
|||
## Let's get to it and meet your new PMC members! | |||
## Let's get to it and meet your new PMC members |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
expressing excitement is perfectly valid
No description provided.