-
-
Notifications
You must be signed in to change notification settings - Fork 78
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
New "module_missing" lint to detect missing, renamed, or non-visible modules #534
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nicely done! All looks correct, just a few tiny nitpicks.
src/lints/module_missing.ron
Outdated
"public": "public", | ||
"zero": 0, | ||
}, | ||
error_message: "A publicly-visible module cannot be imported by its prior path. A `pub use` may have been removed, or the module may have been renamed, removed, or made non-visible.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
error_message: "A publicly-visible module cannot be imported by its prior path. A `pub use` may have been removed, or the module may have been renamed, removed, or made non-visible.", | |
error_message: "A publicly-visible module cannot be imported by its prior path. A `pub use` may have been removed, or the module may have been renamed, removed, or made non-public.", |
Tweak error message.
Adjust formatting on test_crates/module_missing
Pushed the suggested changes. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a minor consistency fix applying the formatting to the commented-out code as well.
Marked for merge as soon as the tests finish, well done and thank you! Are you interested in writing some more lints, or perhaps improving the contributing docs based on which things weren't obvious? No pressure of course, but if you are interested, I'm happy to keep merging :) |
FWIW, I do hope to add documentation, cleanups, and maybe a few more lints, in what free time I have... but of course, that free time is not as abundant as I would like. ;) |
Of course! Any PRs you're interested in putting together are welcome and appreciated, regardless of timeline. You're in charge of the pace, I just want to make sure you are never blocked on me. In terms of next lints, #366 has 3 suggestions that already have the necessary schema and just need lints and test cases, so they are the lowest-hanging fruit. If you'd like something more substantial for lints, we could also use help implementing |
Closes #482
I tried to follow your instructions at #482 and at obi1kenobi/trustfall-rustdoc-adapter#261 (comment) ; how does this look?