feat(cmd): add 'get' subcommand for build and buildrun resources - #412
mohit-bhandari45 wants to merge 1 commit into
Conversation
|
Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@kaizakin Please review this pr. |
Signed-off-by: mohit-bhandari45 <mohitbhandari852@gmail.com>
33a2640 to
8ff0a70
Compare
|
@SaschaSchwarze0 Do review this once you get time. |
kaizakin
left a comment
There was a problem hiding this comment.
please address my comments
and btw try to reduce duplicated code if possible/applicable
| w := tabwriter.NewWriter(ioStreams.Out, 0, 8, 2, '\t', 0); | ||
| fmt.Fprintf(w, "NAME:\t%s\n", build.Name) | ||
| fmt.Fprintf(w, "NAMESPACE:\t%s\n", build.Namespace) | ||
| if build.Spec.Source.Git != nil { |
There was a problem hiding this comment.
please check Spec.Source is not nil before doing this
| build, err := clientset.ShipwrightV1beta1().Builds(ns).Get(c.cmd.Context(), c.name, metav1.GetOptions{}) | ||
| if err != nil { | ||
| if k8serrors.IsNotFound(err) { | ||
| fmt.Fprintf(ioStreams.Out, "Build '%s' not found in namespace '%s'.\n", c.name, ns) |
There was a problem hiding this comment.
i think you should propagate the error
and a non-zero exit
| } | ||
|
|
||
| func (c *GetCommand) Validate() error { | ||
| if c.name == "" { |
There was a problem hiding this comment.
please check for invalid -o values as well
|
|
||
| switch c.output { | ||
| case "json": | ||
| data, err := json.MarshalIndent(build, "", " "); |
There was a problem hiding this comment.
why a semi-colon here?
also please make sure you format the code with gofmt
it would be also better if you run golangci-lint as well
|
|
||
| // TODO: add support for `update` and `get` commands | ||
| command.AddCommand( | ||
| runner.NewRunner(p, ioStreams, getCmd()).Cmd(), |
There was a problem hiding this comment.
remember to generate the docs
there's a makefile target for that
| @@ -23,6 +23,7 @@ func Command(p *params.Params, ioStreams *genericclioptions.IOStreams) *cobra.Co | |||
|
|
|||
| // TODO: add support for `update` and `get` commands | |||
There was a problem hiding this comment.
this TODO is left untouched
update this please
| status := "Unknown" | ||
| for _, condition := range buildRun.Status.Conditions { | ||
| if condition.Type == buildv1beta1.Succeeded { | ||
| status = condition.Reason |
|
|
||
| cmd.Flags().StringVarP(&c.output, "output", "o", "", "Output format. Allowed values: json, yaml") | ||
| return c | ||
| } No newline at end of file |
| if err.Error() != expectedErr { | ||
| t.Errorf("expected error %q, but got %q", expectedErr, err.Error()) | ||
| } | ||
| } No newline at end of file |
|
|
||
| cmd.Flags().StringVarP(&c.output, "output", "o", "", "Output format. Allowed values: json, yaml") | ||
| return c | ||
| } No newline at end of file |
Changes
Introduced the missing
getsubcommand for bothbuildandbuildrunresource managers (shp build getandshp buildrun get):shp build get <name>: Retrieves detailed specification and status for aBuildobject (Source URL, revision, strategy, output image, registration status).shp buildrun get <name>: Retrieves detailed specification and status for aBuildRunexecution object (Build ref, status condition, start & completion times).-o jsonand-o yamlflags, as well as a default clean key-value summary table.NotFounderrors gracefully with friendly messages (e.g.Build 'xyz' not found in namespace 'default'.).pkg/shp/cmd/build/get_test.goandpkg/shp/cmd/buildrun/get_test.go(100% passing).Related Issue
Fixes #411
Type of PR
/kind feature
Submitter Checklist
Release Notes