Dasharo / twpm-docs

Trustworthy Platform Module (TwPM) documentation
https://twpm.dasharo.com
5 stars 0 forks source link

docs/development/verilog_modules.md: new file #7

Closed krystian-hebel closed 1 year ago

miczyg1 commented 1 year ago

Looks fine, waiting for TBDs to be filled

miczyg1 commented 1 year ago

Approved, but the pre-commit fails.

krystian-hebel commented 1 year ago

I've fixed mkdocs.yml on which pre-commit was complaining, now it wants me to add output to cd commands, at the same time complaining that the output of device utilization contains tabs instead of spaces. Output of some commands is several screens long. I'd rather not add it all...

It also doesn't like link (docs/tutorials/building.md:131:12 MD051/link-fragments Link fragments should be valid [Context: "[the easy path](#building-easy)"]) which I had to change to actually work in pages produced by mkdocs. Previous version of that link which works for GFM (#building---easy) doesn't work with mkdocs. Not sure if this is configurable, but in this case it is false-positive. @macpijan CC

macpijan commented 1 year ago

@krystian-hebel

now it wants me to add output to cd command

No, it wants you to remove the $ where it is not necessary, so it is easier to copy commands from documentation. See: https://github.com/updownpress/markdown-lint/blob/master/rules/014-commands-show-output.md

If there are no prompts, you may click the copy button in rendered docs, and paste all of the commands into the terminal with one click. Alternatively, you may separate the commands in to individual steps (via multiple code blocks), if for some reason this would be required. But again, if there would be no prompt, you can use the button to copy the entire command into the buffer.

complaining that the output of device utilization contains tabs instead of spaces

Cannot we use spaces instead of tabs in the output, even if in reality command produced tabs? If you consider this as a problem, you may adjust the rules to exclude code blocks from this check: https://github.com/DavidAnson/markdownlint/blob/main/schema/.markdownlint.yaml#L54

Or maybe set: spaces_per_tab: 1 to e.g. 8 spaces, and the auto formatter would convert the tabs to 8 spaces locally for you, so you can commit that.

Output of some commands is several screens long. I'd rather not add it all...

Then do not add it. Then, the above problem with tabs is gone as well?

It also doesn't like link (docs/tutorials/building.md:131:12 MD051/link-fragments Link fragments should be valid [Context: "the easy path"]) which I had to change to actually work in pages produced by mkdocs. Previous version of that link which works for GFM (#building---easy) doesn't work with mkdocs. Not sure if this is configurable, but in this case it is false-positive.

Please add a comment explaining that and disable the MD051 check for this particular line. Let's see if we have more cases like this in the future, before we disable this check entirely, as in general, it seems valuable to have.

You can disable specific check for the next line with: https://github.com/DavidAnson/markdownlint#configuration

<!-- markdownlint-disable-next-line MD051 -->
macpijan commented 1 year ago

Hmm, I can see that we do not have the lint config commited in this repo, as we have in: https://github.com/Dasharo/docs/blob/master/.markdownlint.yaml Which means we use default settings. I will commit it to the main branch.

macpijan commented 1 year ago

@krystian-hebel I pushed slightly modified linter config to main branch, so you can rebase.