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

Lint markdown docs #6480

Closed
wants to merge 3 commits into from
Closed

Lint markdown docs #6480

wants to merge 3 commits into from

Conversation

joshka
Copy link
Contributor

@joshka joshka commented Apr 12, 2024

  • docs: lint and reformat the main markdown files
  • style: automatically format ci.yaml

Motivation

When saving changes to markdown files in VSCode with a fairly standard setup (markdown lint extension installed),
often the extension will reformat areas of the docs unrelated to the the specific change. This PR makes everything
neat (100 character lines, consistent list characters, code languages on fenced code blocks etc.)

Solution

Reformatted the root level markdown docs
Added a markdown lint config file to set the line length (picked 100 chars as a reasonable compromise. 120 is generally too long for docs, while 80 seems overly constraining).
Added a markdown lint action to the ci.yml - currently this only points at the markdown docs in the root of the project, not those deeper in the project tree, but this could be ratcheted down to cover those once they're also similarly reformatted.

Copy link
Contributor Author

@joshka joshka left a comment

Choose a reason for hiding this comment

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

Some quick explanations

Comment on lines -128 to +113
```
```shell
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In general, I've gone with shell over bash as the commands are relevant generally to any shell.

Comment on lines -268 to 263
```bash
$ cd tokio
$ cargo fuzz list
```shell
cd tokio
cargo fuzz list
```

```text
fuzz_linked_list
````
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a good idea to generally split commands and the output into separate blocks instead of using shell prefixes, to make it clearer that the user shouldn't be pasting the output of the command.


#### Commit message guidelines

A good commit message should describe what changed and why.

1. The first line should:
- contain a short description of the change (preferably 50 characters or less, and no more than
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using - for lists helps avoid the confusion between lists and italic / bold text

Comment on lines -100 to +96
More examples can be found [here][examples]. For a larger "real world" example, see the
The Tokio github repository contains more [examples]. For a larger "real world" example, see the
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Small content change here.

@joshka
Copy link
Contributor Author

joshka commented Apr 12, 2024

https://github.com/tokio-rs/tokio/actions/runs/8668104605/job/23772478137?pr=6481:

markdownlint-cli2 v0.13.0 (markdownlint v0.34.0)
Finding: *.{md,markdown}
Linting: 4 file(s)
Summary: 2 error(s)
Error: README.md:1 MD041/first-line-heading/first-line-h1 First line in a file should be a top-level heading [Context: "## Tokio"] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md041.md
Error: README.md:9:1 MD004/ul-style Unordered list style [Expected: dash; Actual: asterisk] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md004.md
Error: Failed with exit code: 1

Comment on lines -228 to 219
```text
```toml
tokio = { version = "~1.32", features = [...] }
```
Copy link
Contributor

Choose a reason for hiding this comment

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

This is rendered quite badly by github.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. What about one of the following instead?

tokio = { version = "~1.32", features = ["..."] }
tokio = { version = "~1.32", features = ... }

Copy link
Member

Choose a reason for hiding this comment

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

I don't have a strong preference, but the former seems more natural to me?

Copy link
Member

@mox692 mox692 left a comment

Choose a reason for hiding this comment

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

I left some notes regarding the small content changes I found, but all of these seem fine to me.


* **Fast**: Tokio's zero-cost abstractions give you bare-metal
performance.
Tokio is:
Copy link
Member

Choose a reason for hiding this comment

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

content change: It is -> Tokio is

@@ -58,7 +53,8 @@ Make sure you activated the full features of the tokio crate on Cargo.toml:
[dependencies]
tokio = { version = "1.37.0", features = ["full"] }
```
Then, on your main.rs:

Then, in your main.rs:
Copy link
Member

Choose a reason for hiding this comment

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

content change: on your main.rs -> in your main.rs

* `tokio-stream` - [view changelog](https://github.com/tokio-rs/tokio/blob/master/tokio-stream/CHANGELOG.md)
* `tokio-macros` - [view changelog](https://github.com/tokio-rs/tokio/blob/master/tokio-macros/CHANGELOG.md)
* `tokio-test` - [view changelog](https://github.com/tokio-rs/tokio/blob/master/tokio-test/CHANGELOG.md)
- [`tokio` changelog](https://github.com/tokio-rs/tokio/blob/master/tokio/CHANGELOG.md)
Copy link
Member

Choose a reason for hiding this comment

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

content change: view changelog -> changelog


`cargo install cargo-fuzz`
```shell
cargo install --locked cargo-fuzz
Copy link
Member

Choose a reason for hiding this comment

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

content change: cargo install cargo-fuzz -> cargo install --locked cargo-fuzz


* **Fast**: Tokio's zero-cost abstractions give you bare-metal
performance.
Tokio is:
Copy link
Member

Choose a reason for hiding this comment

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

content change: It is -> Tokio is

@@ -58,7 +53,8 @@ Make sure you activated the full features of the tokio crate on Cargo.toml:
[dependencies]
tokio = { version = "1.37.0", features = ["full"] }
```
Then, on your main.rs:

Then, in your main.rs:
Copy link
Member

Choose a reason for hiding this comment

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

content change: on your main.rs -> in your main.rs

@mox692 mox692 added T-docs Topic: documentation A-readme Area: Documentation that isn't part of any crate such as README.md or CONTRIBUTING.md labels Apr 16, 2024
@Darksonn
Copy link
Contributor

Darksonn commented May 1, 2024

I don't actually want to do this. We require code to pass rustfmt because otherwise we get too many PRs with badly formatted code. We don't have this problem with markdown, and I prefer the freedom that we currently have.

Either way, thank you for taking the time to submit a PR.

@Darksonn Darksonn closed this May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-readme Area: Documentation that isn't part of any crate such as README.md or CONTRIBUTING.md T-docs Topic: documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants