-
Notifications
You must be signed in to change notification settings - Fork 244
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
[Devfile] Remove namespace flag from odo commands #2909
[Devfile] Remove namespace flag from odo commands #2909
Conversation
Signed-off-by: John Collier <John.J.Collier@ibm.com>
Signed-off-by: John Collier <John.J.Collier@ibm.com>
Signed-off-by: John Collier <John.J.Collier@ibm.com>
Signed-off-by: John Collier <John.J.Collier@ibm.com>
/retest More OpenShift CI flakiness |
/retest |
Codecov Report
@@ Coverage Diff @@
## master #2909 +/- ##
=======================================
Coverage 44.71% 44.71%
=======================================
Files 107 107
Lines 10126 10126
=======================================
Hits 4528 4528
Misses 5162 5162
Partials 436 436 Continue to review full report at Codecov.
|
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kadel 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 |
Signed-off-by: John Collier <John.J.Collier@ibm.com>
pkg/odo/cli/component/push.go
Outdated
@@ -65,6 +66,13 @@ func (po *PushOptions) Complete(name string, cmd *cobra.Command, args []string) | |||
} | |||
po.EnvSpecificInfo = envinfo | |||
po.Context = genericclioptions.NewDevfileContext(cmd) | |||
|
|||
if cmd.Flags().Changed("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.
I think this needs to be moved here https://github.com/openshift/odo/blob/6740e5177ae11db734f7d3d303c4f689015e3b91/pkg/odo/genericclioptions/context.go#L343. We are repeating the same steps in all the above files.
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.
@mik-dass You're right, good catch!
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.
Actually, we don't need to move any code to resolveNamespace
, just update it to retrieve the namespace from the --project
flag.
So long as we initialize the devfile context in each command that needs it, we can retrieve the namespace via kclient.Namespace
Signed-off-by: John Collier <John.J.Collier@ibm.com>
Signed-off-by: John Collier <John.J.Collier@ibm.com>
Signed-off-by: John Collier <John.J.Collier@ibm.com>
/retest |
1 similar comment
/retest |
Signed-off-by: John Collier <John.J.Collier@ibm.com>
Signed-off-by: John Collier <John.J.Collier@ibm.com>
Signed-off-by: John Collier <John.J.Collier@ibm.com>
/retest |
More CI infra flakes 😢 /retest |
Signed-off-by: John Collier <John.J.Collier@ibm.com>
/retest Giving the builds a kick due to the gosec issues earlier |
/retest |
1 similar comment
/retest |
@mik-dass I've rebased and addressed your review comment (retrieving namespace from the getNamespace function) |
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. |
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. |
There may be a potential merge conflict with this and #2922, so I'm holding this so that it doesn't slip in to this afternoon's release and delay 2922. I'll cancel the hold after the release. /hold |
Signed-off-by: John Collier <John.J.Collier@ibm.com>
Signed-off-by: John Collier <John.J.Collier@ibm.com>
/hold cancel |
tests were updated! Thx |
/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. |
What type of PR is this?
/kind cleanup
/area devfile
What does does this PR do / why we need it:
As mentioned earlier, we decided to use only the
--project
flag in odo for devfile components and to not use--namespace
at this moment. This PR removes the--namespace
flag that was added to certain odo commands for use with devfile components and uses--project
instead. For commands that didn't have the--project
flag added to it, the flag was added.Integration tests updated to use
--project
only as well.Which issue(s) this PR fixes:
Fixes #2769
How to test changes / Special notes to the reviewer:
odo create --project <project>
-> Verify namespace/project in env.yaml is correctodo push --project <project>
-> should push to the proper namespaceodo delete --project <project>
-> should delete from the proper namespaceodo watch --project <project>
-> should push changes to the proper namespace