-
Notifications
You must be signed in to change notification settings - Fork 243
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
Rename --downloadSource to --starter #3425
Rename --downloadSource to --starter #3425
Conversation
Signed-off-by: John Collier <John.J.Collier@ibm.com>
/hold til #3291 is merged |
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.
/lgtm
/retest |
/hold cancel Ready for approver review |
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.
/approve
/retest Please review the full test history for this PR and help us cut down flakes. |
2 similar comments
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
@@ -99,7 +99,7 @@ In this example we will be deploying an https://github.com/odo-devfiles/springbo | |||
---- | |||
$ git clone https://github.com/odo-devfiles/springboot-ex | |||
---- | |||
Alternatively, you can pass in `--downloadSource` to `odo create` to have odo download a sample project. | |||
Alternatively, you can pass in `--starter` to `odo create` to have odo download a sample project. |
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.
Encountered an issue with documentation this morning when using the previously named --downloadSource
functionality. I think that we should change the wording here to something along the lines of:
to have odo download the specified repository under projects
The reason being is that when I used --help
it talked about the sample project, which is incorrect, as it's pulling from the projects
section of devfile.yaml
, so it's a tad confusing.
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.
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.
How about download the sample project specified in the devfile
?
pkg/odo/cli/component/create.go
Outdated
@@ -1079,8 +1079,8 @@ func NewCmdCreate(name, fullName string) *cobra.Command { | |||
componentCreateCmd.Flags().StringSliceVar(&co.componentEnvVars, "env", []string{}, "Environmental variables for the component. For example --env VariableName=Value") | |||
|
|||
if experimental.IsExperimentalModeEnabled() { | |||
componentCreateCmd.Flags().StringVar(&co.devfileMetadata.downloadSource, "downloadSource", "", "Download sample project from devfile.") | |||
componentCreateCmd.Flags().Lookup("downloadSource").NoOptDefVal = defaultProjectName //Default value to pass to the flag if one is not specified. | |||
componentCreateCmd.Flags().StringVar(&co.devfileMetadata.starter, "starter", "", "Download starter project from devfile.") |
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.
Honestly, so weird using this wording. Because if you try to do odo create
with just: --starter
and --devfile
, example:
odo create --starter --devfile https://github.com/odo-devfiles/nodejs-ex/archive/0.0.1.zip
It makes it seem like it's downloading a "sample" project, but it's not, it's downloading the actual project specified in projects
under the devfile.yaml
file, if that makes sense?
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.
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.
@cdrage I agree with you this needs to be fixed. But that is a separate issue.
This is just about renaming this flag. It is supposed to download the sample (starter) project from the devfile the original name was creating a lot of confusion. Once the startProject
field is added to devfile spec we can fix the rest.
Codecov Report
@@ Coverage Diff @@
## master #3425 +/- ##
==========================================
- Coverage 46.51% 46.50% -0.01%
==========================================
Files 112 112
Lines 11223 11223
==========================================
- Hits 5220 5219 -1
+ Misses 5504 5503 -1
- Partials 499 501 +2
Continue to review full report at Codecov.
|
Signed-off-by: John Collier <John.J.Collier@ibm.com>
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mik-dass The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
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.
/lgtm
/retest Please review the full test history for this PR and help us cut down flakes. |
3 similar comments
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
11 similar comments
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
Signed-off-by: John Collier <John.J.Collier@ibm.com>
/retest |
/lgtm |
/retest Please review the full test history for this PR and help us cut down flakes. |
5 similar comments
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
Signed-off-by: John Collier John.J.Collier@ibm.com
What type of PR is this?
/kind cleanup
What does does this PR do / why we need it:
This PR renames the
--downloadSource
flag onodo create
to--starter
, as decided in #3107. Related integration tests and documentation have also been updated.Which issue(s) this PR fixes:
Fixes #3107
How to test changes / Special notes to the reviewer:
Verify
odo create <component> --starter
works as expected, andodo create <component> --downloadSource
no longer works