-
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
odo describe component #5725
odo describe component #5725
Conversation
✅ Deploy Preview for odo-docusaurus-preview canceled.
|
9d68bb9
to
5687b1d
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.
The changes look good, I only have a few questions/suggestions.
pkg/odo/cli/describe/component.go
Outdated
return api.Component{}, err | ||
return api.Component{}, nil, err | ||
} | ||
devfile, err := component.GetDevfileInfoFromCluster(o.clientset.KubernetesClient, name, o.clientset.KubernetesClient.GetCurrentNamespace()) |
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.
If you're going to be passing o.clientset.KubernetesClient
, why not just pass that and calculate current namespace in the function? I think you can drop o.clientset.KubernetesClient.GetCurrentNamespace()
arg, no?
Also, same comment for L110.
288c4e3
to
52a79c6
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.
Thank you for working on those changes, Philippe!
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, but I had a few small questions (might be enhancements for other PRs) :)
-
In the "Running in" section, do we want to be more specific with the "Debug" Dev mode? Because the command displays "Dev" no matter if I run
odo dev
orodo dev --debug
.
I found it a bit strange given that the "Supported odo features" section clearly lists "Debug" as a possible feature supported, but this is currently not listed in "Running in" when running in this mode. -
Also, I understood that the "Supported odo features" information is determined from the current Devfile and currently displays "Unknown" when running with
--name
. When running with the--name
flag, if a mode (Dev
and/orDeploy
) is detected and listed in the "Running in" section, it means that this feature is supported byodo
, no?
❯ odo describe component --name my-app
Name: my-app
Display Name: Unknown
Project Type: quarkus
Language: Unknown
Version: Unknown
Description: Unknown
Tags:
Running in: Dev, Deploy
Supported odo features:
• Dev: Unknown
• Deploy: Unknown
• Debug: Unknown
@kadel That's a good point, do we want to make the difference between the Dev and Debug modes for the running modes?
I think the odo supported features are mostly intended to indicate that we can run the command "odo dev/deploy". Even if the component has been deployed, doesn't mean we can run it now (in this case, we couldn't, because we don't have the Devfile) |
Okay, I see. Thanks. |
Co-authored-by: Parthvi Vala <pvala@redhat.com>
Co-authored-by: Parthvi Vala <pvala@redhat.com>
Co-authored-by: Parthvi Vala <pvala@redhat.com>
52a79c6
to
edf6332
Compare
4a31498
to
cfdff49
Compare
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
/lgtm |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rm3l 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 |
* odo describe component * More fields on named describe * Doc * Update pkg/odo/cli/describe/component.go Co-authored-by: Parthvi Vala <pvala@redhat.com> * Update pkg/odo/cli/describe/component.go Co-authored-by: Parthvi Vala <pvala@redhat.com> * Update pkg/odo/cli/describe/component.go Co-authored-by: Parthvi Vala <pvala@redhat.com> * Add Describef * Parthvi review * Fix rebase Co-authored-by: Parthvi Vala <pvala@redhat.com>
What type of PR is this:
/kind feature
What does this PR do / why we need it:
Which issue(s) this PR fixes:
Fixes #5661
PR acceptance criteria:
Unit test
Integration test
Documentation
How to test changes / Special notes to the reviewer:
Doc preview: https://deploy-preview-5725--odo-docusaurus-preview.netlify.app/docs/3.0.0/command-reference/describe