-
Notifications
You must be signed in to change notification settings - Fork 46
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
Merge get/list env command #206
Merge get/list env command #206
Conversation
@SarthakJain26 its hard to just add a flag and show the whole details. Like not hard but unnecessary cause it will make the command slower if we query all the fields for list environments also. We should consider adding it to descibe or something. WDYT? |
i havent touched other part of the code which can be optimised with this new get environment endpoint, we would do that in some other pr. I will create an issue regarding that. |
|
pkg/cmd/get/environments.go
Outdated
} | ||
} | ||
|
||
environmentList, err := environment.ListChaosEnvironments(projectID, credentials) |
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.
Shouldn't we have a check here to see if environmentID
is given or not before sending the query
pkg/cmd/get/get.go
Outdated
@@ -35,6 +35,9 @@ var GetCmd = &cobra.Command{ | |||
#get list of Chaos Experiment runs | |||
litmusctl get chaos-experiment-runs --project-id="" | |||
|
|||
#get list of Chaos Environments | |||
litmusctl get chaos-experiments --project-id="" |
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.
add example for environment-id also
@Nageshbansal does the master branch have updated-dependencies? Not rebasing in case, dont want to add extra commit |
Not yet, as Pr#209 is still opened. |
a099141
to
25e3fd3
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.
LGTM, geat work 🚀
Signed-off-by: Shivam Purohit <[email protected]>
Signed-off-by: Shivam Purohit <[email protected]>
Signed-off-by: Shivam Purohit <[email protected]>
25e3fd3
to
49af013
Compare
Merges both
get chaos-environment
andlist chaos-envrionments
commands into a single command asget chaos-environments
.