Fix redundant_readme lint for lower-priority README files#17208
Fix redundant_readme lint for lower-priority README files#17208goutamadwant wants to merge 1 commit into
Conversation
rustbot
added
A-manifest
|
Thanks for the pull request, and welcome! The Rust Project is excited to review your changes, and you should hear from @epage (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
Why was this reviewer chosen?The reviewer was selected based on:
|
| }; | ||
|
|
||
| if !DEFAULT_README_FILES.contains(&readme.as_str()) { | ||
| if default_readme_from_package_root(pkg.root()).as_deref() != Some(readme) { |
There was a problem hiding this comment.
Should we keep the check of DEFAULT_README_FILES as a fast path?
This comment has been minimized.
This comment has been minimized.
78244e6 to
1ee9f2d
Compare
This comment has been minimized.
This comment has been minimized.
|
@goutamadwant thanks, though you might want to hold off until #17231 gets merged. |
This comment has been minimized.
This comment has been minimized.
1ee9f2d to
6d32d91
Compare
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
@weihanglo I see the PR #17231 merged and there were merge conflicts too - resolved them now, Let me know. Thanks! |
| #[cargo_test] | ||
| fn lower_priority_readme() { |
There was a problem hiding this comment.
Sorry, forgot to call out our contrib guide encourages a specific commit structure with tests where a commit is added with the test, showing a reproduction of the undesired behavior. The follow up commit with the fix then also updates the test to show the new behavior.
4 participants
What does this PR try to resolve?
Closes #17206.
cargo::redundant_readmewarned wheneverpackage.readmematched any default README filename. That can be wrong when a higher-priority default README file also exists. For example, removingreadme = "README.txt"whileREADME.mdis present would make Cargo inferREADME.md, changing the package metadata.This changes the lint to compare the explicit readme value with the same package-root README inference helper used by manifest normalization, so the lint only fires when removing the field preserves the inferred README.
How to test and review this PR?
Added coverage for both sides of the inference behavior:
README.txtwithREADME.mdalso present should not warn.README.txtas the inferred default should still warn.Commands run:
cargo fmt --checkcargo test -p cargo --test testsuite -- lints::redundant_readmecargo test -p cargo --test testsuite -- lints::