-
Notifications
You must be signed in to change notification settings - Fork 24
feat: accept the app name as an argument to the 'samples' command #140
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
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #140 +/- ##
==========================================
+ Coverage 63.54% 63.59% +0.04%
==========================================
Files 212 212
Lines 22395 22398 +3
==========================================
+ Hits 14232 14244 +12
+ Misses 7077 7072 -5
+ Partials 1086 1082 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
mwbrooks
left a comment
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.
✅ Looks great and this is a really nice improvement that was overlooked!
🧪 Manual testing works well and test coverage looks good.
❓ Thoughts on this being a feature rather than a bug? Our command's help clearly documented that there were no arguments and the logic prevented extra arguments. To me, this looks like an enhancement and semver:minor 😄
| if len(args) > 0 { | ||
| createCmd.SetArgs([]string{args[0]}) | ||
| } | ||
| createCmd.SetArgs(args) |
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.
praise: Ah, much more elegant!
|
@mwbrooks Thanks so much - I was confused to find this didn't match the But I notice now that documentation reflected what was found in releases so I updated this to be a "feat" and will merge it as such! 🚢 💨 |
Summary
This PR accepts the app name argument for the
samplescommand.Preview
samples.mov
Reviewers
Create even more samples at specified paths or not with these commands:
Requirements