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

Cleaning up documentation #876

Merged
merged 2 commits into from
May 27, 2023
Merged

Cleaning up documentation #876

merged 2 commits into from
May 27, 2023

Conversation

nyurik
Copy link
Contributor

@nyurik nyurik commented May 26, 2023

Applied this lint:

just 'clippy-flags=-W clippy::doc-markdown' clippy-fix

I also had to search/replace:

[`to_decimal`()]  ->  [`to_decimal()`]

I have never seen parenthesis used as part of the link, so this is perhaps why clippy handles it like that.

There are still a few warnings being produced, but they have to do with how some of the links are constructed, e.g. this link

[File::index_names().len()][File::index_names()]

shows a warning

you should put bare URLs between </> or make a proper Markdown link

Not certain how these should be fixed, but there are 10-20 of such warnings. What are you linking to here?

Applied this lint:

```
just 'clippy-flags=-W clippy::doc-markdown' clippy-fix
```

I also had to search/replace:

```
[`to_decimal`()]  ->  [`to_decimal()``]
```

I have never seen parenthesis used as part of the link, so this is perhaps why clippy handles it like that.

There are still a few warnings being produced, but they have to do with how some of the links are constructed, e.g. this link

```
[File::index_names().len()][File::index_names()]
```

shows a warning

> you should put bare URLs between `<`/`>` or make a proper Markdown link

Not certain how these should be fixed, but there are 10-20 of such warnings.  What are you linking to here?
Copy link
Member

@Byron Byron left a comment

Choose a reason for hiding this comment

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

Incredible, I wasn't aware of all the tricks clippy has up its sleeves, and appreciate all the improvements that are made by it (and the industrious human who controls all that, thank you :)).

I managed to find a couple of other issues around these function(<ellipsis>) doc strings, and am happy about these massive improvements.

@nyurik
Copy link
Contributor Author

nyurik commented May 27, 2023

thx @Byron ! Do you actually want to include parens everywhere in the docs? I haven't seen many usages like that. Most of the time I see either [Some::Path::to_function] or the Markdown style links like [...text...](...link...). In both cases they tend to be wrapped in backticks

@Byron
Copy link
Member

Byron commented May 27, 2023

The reason I do this is that I'd like to differentiate between fields and methods. Admittedly, most of the time the name takes care of that, but I felt like not taking chances.

If the current style disturbs tooling too much, I think that's enough to remove (…) and () in links even though I'd love to have it styled so that their nature is clear.

@Byron Byron merged commit 420553a into GitoxideLabs:main May 27, 2023
@nyurik nyurik deleted the fix-docs branch May 27, 2023 20:26
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.

2 participants