-
Notifications
You must be signed in to change notification settings - Fork 880
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
Introduce kubectl argo rollouts list experiments
#267
Conversation
Codecov Report
@@ Coverage Diff @@
## master #267 +/- ##
==========================================
+ Coverage 83.54% 83.68% +0.14%
==========================================
Files 61 63 +2
Lines 5487 5548 +61
==========================================
+ Hits 4584 4643 +59
- Misses 664 665 +1
- Partials 239 240 +1
Continue to review full report at Codecov.
|
36a11f2
to
5b97228
Compare
5b97228
to
7d45bc1
Compare
ExperimentKind string = "Experiment" | ||
ExperimentSingular string = "experiment" | ||
ExperimentPlural string = "experiments" | ||
ExperimentFullName string = ExperimentPlural + "." + Group |
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.
We were no longer using the shortnames so i removed it. instead i added it as kubebuilder comments
@@ -0,0 +1,158 @@ | |||
package list |
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.
This file mainly lifted and shifted from list.go
@@ -95,44 +96,3 @@ func (ri *rolloutInfo) String(timestamp, namespace bool) string { | |||
} | |||
return fmt.Sprintf(fmtString, 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.
Got rid of rolloutStauts in favor of the one in info package since there was duplication and inconsistencies in logic.
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
ExperimentKind string = "Experiment" | ||
ExperimentSingular string = "experiment" | ||
ExperimentPlural string = "experiments" | ||
ExperimentShortName string = "ex" |
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.
Why do you remove the short names?
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 explained above -- we werent using it anymore. And the kubebuilder annotations is the source of truthe.
@@ -151,8 +153,6 @@ type AnalysisRunSpec struct { | |||
AnalysisSpec AnalysisTemplateSpec `json:"analysisSpec"` | |||
// Arguments hold the arguments to the run to be used by metric providers | |||
Arguments []Argument `json:"arguments,omitempty"` | |||
// ReplicaSets identifies the ReplicaSets in which to monitor to decide when to begin analysis | |||
ReplicaSets []string `json:"replicaSets,omitempty"` |
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.
Are you removing these since we aren't using them?
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.
Yes.
Built ontop of #266. Only look at last commit.