-
Notifications
You must be signed in to change notification settings - Fork 23
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: enable building without project type specification #222
Conversation
Codecov ReportAttention: Patch coverage is
@@ Coverage Diff @@
## main #222 +/- ##
==========================================
+ Coverage 70.34% 70.40% +0.06%
==========================================
Files 44 46 +2
Lines 6787 7045 +258
Branches 6787 7045 +258
==========================================
+ Hits 4774 4960 +186
- Misses 1240 1258 +18
- Partials 773 827 +54
|
c30a4d8
to
855820c
Compare
855820c
to
20ee151
Compare
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.
Reviewed. LGTM.
As agreed separately with PO.
Simply attempts to build as a normal Rust project otherwise.
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.
Great job. I just left a few small comments. This will have a lot of conflicts with: #219. If you finish the test, I am happy to merge this first and handle the conflicts myself.
There is no need to include it in this PR. But as a refactor we should remove the short = 'p' in the other commands to maintain consistency and avoid confusing the user.
#[arg(short = 'p', long)]
path: Option<PathBuf>,
And finally, this PR will close #91
# Conflicts: # crates/pop-contracts/src/errors.rs
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.
Great!! LGTM
Enables building without needing to specify the project type - i.e.
pop b -r
TODO:
-p
to support package specification, so it can act as a simple wrapper tocargo b -p