-
Notifications
You must be signed in to change notification settings - Fork 3
refactor: change project layout to make it more Rust. #4
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
Conversation
|
Hi @yyc12345. Thanks for your PR. I'm waiting for a linuxdeepin member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
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.
Hey @yyc12345 - I've reviewed your changes - here's some feedback:
- Consider relocating
linguist_file.rsto a more specific module (e.g., undersubcmdor a newutilsmodule) if its functionality isn't broadly used across the library. - The new comment regarding Windows EOL issues in tests raises a question about intended platform support; please clarify if this limitation is acceptable for the project.
- Consider consistently applying the new Rustdoc comment style (mentioned in the PR description) to other structs and enums modified or introduced in this refactor, such as those in
cli.rs.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Reviewer's GuideThis pull request refactors the project's structure to align with Rust best practices. Key changes include introducing File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
- change project layout to make it suit for Rust best practices. * add `lib.rs` to manage modules, rather than declare them in `main.rs`. * use module folder to manage project. - rename `cli_args` to `cli` to make it correspond with its meaning (not only cli arguments, but also all things involving cli). - move cli executor into `cli` module to make `main.rs` be clean. - use Rust document syntax to make comments for struct and fields, instead of normal comments. - fix an unittest which fail on Windows due to EOL issue (CRLF/LF error).
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: BLumia, yyc12345 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
lib.rsto manage modules, rather than declare them inmain.rs.cli_argstoclito make it correspond with its meaning (not only cli arguments, but also all things involving cli).climodule to makemain.rsbe clean.Summary by Sourcery
Refactor the project structure to follow Rust best practices by reorganizing modules, improving code organization, and enhancing documentation.
New Features:
lib.rsto manage project modulesEnhancements:
cli_argsmodule toclifor broader scopeChores: