Skip to content

Remove unused --config option#139

Merged
knative-prow-robot merged 3 commits intoknative:masterfrom
nak3:remove-config
Jun 8, 2019
Merged

Remove unused --config option#139
knative-prow-robot merged 3 commits intoknative:masterfrom
nak3:remove-config

Conversation

@nak3
Copy link
Copy Markdown
Contributor

@nak3 nak3 commented May 22, 2019 •
edited
Loading

Currently parameters read by --config in kn can not be used.
This patch removes the option from the command.

Fixes #138

@knative-prow-robot knative-prow-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label May 22, 2019
@knative-prow-robot
Copy link
Copy Markdown
Contributor

knative-prow-robot commented May 22, 2019

Hi @nak3. Thanks for your PR.

I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions 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/test-infra repository.

@knative-prow-robot knative-prow-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label May 22, 2019
@cppforlife
Copy link
Copy Markdown

cppforlife commented May 28, 2019

@sixolet i believe you've added this initially. any thoughts on usefulness of this feature?

rhuss
rhuss approved these changes May 30, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@rhuss rhuss left a comment

Choose a reason for hiding this comment

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

lgtm and I think we should remove it now even if we should add it back later (maybe for preferences of default displays options wrt to color/compact for human-readable output, like suggested in #75).

But for now I don't see a need for a config file yet.

@rhuss rhuss mentioned this pull request Jun 2, 2019
Closed
@cppforlife
Copy link
Copy Markdown

cppforlife commented Jun 3, 2019

/ok-to-test

@knative-prow-robot knative-prow-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jun 3, 2019
@cppforlife
Copy link
Copy Markdown

cppforlife commented Jun 3, 2019

/lgtm
/approve

@knative-prow-robot knative-prow-robot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jun 3, 2019
@knative-metrics-robot
Copy link
Copy Markdown

knative-metrics-robot commented Jun 3, 2019

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

File Old Coverage New Coverage Delta
pkg/kn/commands/root.go 34.7% 44.4% 9.8

@rhuss
Copy link
Copy Markdown
Contributor

rhuss commented Jun 3, 2019

@nak3 I guess you need an dependency update / rebase for the tests to pass.
@cppforlife thanks for taking care !

@knative-prow-robot knative-prow-robot removed the lgtm Indicates that a PR is ready to be merged. label Jun 3, 2019
@googlebot googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Jun 3, 2019
@knative-prow-robot knative-prow-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 3, 2019
@knative-metrics-robot
Copy link
Copy Markdown

knative-metrics-robot commented Jun 3, 2019

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

File Old Coverage New Coverage Delta
pkg/kn/commands/root.go 34.7% 44.4% 9.8

@nak3
Copy link
Copy Markdown
Contributor Author

nak3 commented Jun 3, 2019

@rhuss @cppforlife Thank you! Updated and it looks like all tests were passed.

By the way, I found current test/presubmit-test.sh updates some files. I filed it as separated issue here #163

@sixolet
Copy link
Copy Markdown
Contributor

sixolet commented Jun 7, 2019

It was there because it was part of the default cobra boilerplate. We can always reintroduce if we need it.
/lgtm
/approve

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 7, 2019
@knative-prow-robot
Copy link
Copy Markdown
Contributor

knative-prow-robot commented Jun 7, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cppforlife, nak3, rhuss, sixolet

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details 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

nak3 added 3 commits June 7, 2019 11:04
@nak3
Remove unused --config option
7ba584d 
Currently `--config` option in kn command is not used from anywhere.
This patch removes the option from the command.
@nak3
update auto gen files
80916b2
@nak3
Update deps by hack/update-deps.sh
42e1566
@knative-prow-robot knative-prow-robot removed the lgtm Indicates that a PR is ready to be merged. label Jun 7, 2019
@nak3
Copy link
Copy Markdown
Contributor Author

nak3 commented Jun 7, 2019

@sixolet @rhuss @cppforlife Thank you. Rebased (so dropped the LGTM label).

@maximilien
Copy link
Copy Markdown
Contributor

maximilien commented Jun 7, 2019

How is this not merged yet when it's been /lgtm by both @cppforlife and @sixolet ?

I see @knative-prow-robot removed the labels. You all need to comment again and /approve.

@sixolet
Copy link
Copy Markdown
Contributor

sixolet commented Jun 8, 2019

/lgtm

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 8, 2019
@knative-prow-robot knative-prow-robot merged commit 1b471d5 into knative:master Jun 8, 2019
@nak3 nak3 mentioned this pull request Jun 9, 2019
Merged
@rhuss rhuss added this to the v0.2.0 milestone Jun 9, 2019
sixolet added a commit to sixolet/client that referenced this pull request Jun 19, 2019
@sixolet
Revert knative#173 and knative#139
257e6d4
knative-prow-robot pushed a commit that referenced this pull request Jun 20, 2019
@sixolet @knative-prow-robot
Revert #173 and #139, reinstating having a config file. (#193)
32ccc33 
* Revert #173 and #139

* Change default layout to have a hidden kn dir

* Update deps
maximilien pushed a commit to maximilien/client that referenced this pull request Jul 1, 2019
@sixolet @maximilien
Revert knative#173 and knative#139, reinstating having a config file. (…
837fb44 
…knative#193)

* Revert knative#173 and knative#139

* Change default layout to have a hidden kn dir

* Update deps
dsimansk added a commit to dsimansk/client that referenced this pull request May 18, 2023
@dsimansk
[release-v1.8] Add func v1.10.0 (knative#139)
e27ee18 
* [release-v1.8] Add func v1.10.0

* Update vendor dir

* Fix pkg name in ldflags
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@evankanderson evankanderson Awaiting requested review from evankanderson

@sixolet sixolet Awaiting requested review from sixolet

1 more reviewer

@rhuss rhuss rhuss approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@cppforlife cppforlife

@sixolet sixolet

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. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Milestone

v0.2.0

Development

Successfully merging this pull request may close these issues.

Unknown usage for --config option

8 participants

@nak3 @knative-prow-robot @cppforlife @knative-metrics-robot @rhuss @sixolet @maximilien @googlebot