Skip to content
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

Upgrade spf13/cobra dep to current master #944

Merged
merged 4 commits into from
Jul 22, 2020

Conversation

navidshaikh
Copy link
Collaborator

Description

  • Pin cobra at v1.0.1-0.20200715031239-b95db644ed1c

  • Parse args without invoking a separate command

    Do not define and execute extractCommand before running actual root
    command to parse all the args without flags. This was creating issues
    with completion utils to generate additional shell completion directive.
    Now uses cobra's inbuilt utilities to parse the command args without flags.

 which includes the zsh completion fix for spf13/cobra#899
 and remove the fork of cobra with fix in `replace` section of go.mod
 Do not define and execute extractCommand before running actual root
 command to parse all the args without flags. This was creating issues
 with completion utils to generate additional shell completion directive.
 Now uses cobra's inbuilt utilities to parse the command args without flags.
@googlebot googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Jul 21, 2020
@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: navidshaikh

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@knative-prow-robot knative-prow-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jul 21, 2020
Copy link
Contributor

@maximilien maximilien left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some feedback. It's questionable if tests for main.go is needed... so do what you can.

Thanks for keeping us up to date with latest Cobra. Best.

cmd/kn/main.go Show resolved Hide resolved
cmd/kn/main.go Show resolved Hide resolved
cmd/kn/main.go Show resolved Hide resolved
cmd/kn/main.go Outdated Show resolved Hide resolved
@knative-metrics-robot
Copy link

The following is the coverage report on the affected files.
Say /test pull-knative-client-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
cmd/kn/main.go 75.9% 75.7% -0.2

@navidshaikh
Copy link
Collaborator Author

/retest

      result_collector.go:75: ERROR: Error: ReconcileIngressFailed: Ingress reconciliation failed

@maximilien
Copy link
Contributor

OK thanks.

/lgtm

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 22, 2020
@knative-prow-robot knative-prow-robot merged commit 8232c9d into knative:master Jul 22, 2020
@navidshaikh navidshaikh deleted the pr/cobra-upgrade branch July 23, 2020 05:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cla: yes Indicates the PR's author has signed the CLA. lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants