-
-
Notifications
You must be signed in to change notification settings - Fork 73
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
feat(cli): Implement mun new
and mun init
#246
Conversation
Codecov Report
@@ Coverage Diff @@
## master #246 +/- ##
==========================================
+ Coverage 78.86% 78.91% +0.05%
==========================================
Files 196 201 +5
Lines 12626 12667 +41
==========================================
+ Hits 9957 9996 +39
- Misses 2669 2671 +2
Continue to review full report at Codecov.
|
Note: I decided to not initialize git to keep this PR as simple as possible. I will probably add that feature in a later PR. |
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.
Thanks for this PR! The functionality looks good to me! Doing the author detection and source control stuff in another PR also sounds great! creating a crate for author detection is even better! Give back to the community when you can. :)
There are a few changes that I would like to see:
-
The Conventional Commit convention states to add the scope between parenthesis, there is also no need for a
.
at the end, and the first letter should be lowercased. Sofeat: mun-new: Add mun new subcommand.
should befeat(mun-new): add mun new subcommand
. Could you modify your commit messages? -
Your changes have a coverage of 8%. Could you maybe add a test to validate that after a
new
orinit
a project is actually created?
Finally, a suggestion:
- It might be better to move some of the functionality to a new file.
Done. |
mun new
and mun init
mun new
and mun init
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.
Thanks for making improvements so quickly. I found one more typo in your latest changes. Other than that it looks good as far as I am concerned.
I'll also leave this for @baszalmstra to approve as he requested some more changes than me. I would ask you to clean up the commit history. Our contributing guide explains how you can do this, but feel free to directly ping me if you have any questions!
Yes! Looks good to me as well, if you can clean up your history a little bit, Ill merge this. :) |
Done |
Thanks for rebasing. A much cleaner history now; very pleasing 🙂 |
Relates to #203 |
closes #237
The
cargo
implementation was used as a reference.Note
I did not do anything to fetch information about the project author.
Cargo uses the function https://github.com/rust-lang/cargo/blob/master/src/cargo/ops/cargo_new.rs#L773.
I could copy it over almost line for line but that feels a little weird. Maybe I should create a small crate for it. Do you think that would be a good idea?