Skip to content

Conversation

@weihanglo
Copy link
Member

What does this PR try to resolve?

Instead we read and concat all lint docs via xtask-lint-docs

How to test and review this PR?

While the size of lints.md is pretty neglectable,
I found it is unnecessary to embed those though.
It is also easier for us to edit and preview the markdown source.

@rustbot rustbot added A-documenting-cargo-itself Area: Cargo's documentation A-infrastructure Area: infrastructure around the cargo repo, ci, releases, etc. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 8, 2025
@rustbot
Copy link
Collaborator

rustbot commented Dec 8, 2025

r? @ehuss

rustbot has assigned @ehuss.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@weihanglo
Copy link
Member Author

cc @Muscraft

Instead we read and concat all lint docs via xtask-lint-docs
@epage
Copy link
Contributor

epage commented Dec 8, 2025

I found it is unnecessary to embed those though.
It is also easier for us to edit and preview the markdown source.

This was done in imitation of clippy.

  • It is nice to have everything for a lint in one spot
    • They used doc comments which help but that was in using a macro which we were avoiding
  • If we do this, we should be mindful of the experience of adding a new lint
    • When blessing the existing lint output, we should write out a template to the expectation locations when files are missing
    • The error message should be more workflow focused
    • We need a check for unreferenced lint pages
    • We need this documented in a process for adding lints

@weihanglo
Copy link
Member Author

This was done in imitation of clippy.

* It is nice to have everything for a lint in one spot
  
  * They used doc comments which help but that was in using a macro which we were avoiding

* If we do this, we should be mindful of the experience of adding a new lint
  
  * When blessing the existing lint output, we should write out a template to the expectation locations when files are missing
  * The error message should be more workflow focused
  * We need a check for unreferenced lint pages
  * We need this documented in a process for adding lints

Except having everything in one spot, the current approach doesn't really achieve any of those. Still relying on code review.

I can add those, though before that we should make sure this is what we want.

@epage
Copy link
Contributor

epage commented Dec 8, 2025

Except having everything in one spot, the current approach doesn't really achieve any of those. Still relying on code review.

What do you mean?

@weihanglo
Copy link
Member Author

Before this, we don't have a lint doc template and documented workflow either. And IMO the developer experience is worse in a sense that you might mess up markdown syntax and don't realize until running cargo lint-docs (though people should always run it before submitting a PR).

Anyway, I a fine with the thing we have today, if we don't agree on the proposed direction. It doesn't block anything.

@rustbot
Copy link
Collaborator

rustbot commented Dec 8, 2025

☔ The latest upstream changes (possibly 14116aa) made this pull request unmergeable. Please resolve the merge conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-documenting-cargo-itself Area: Cargo's documentation A-infrastructure Area: infrastructure around the cargo repo, ci, releases, etc. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants