search

knative / client-contrib

Community contributed `kn` plugins.
Apache License 2.0
10 stars 23 forks source link

feature: Initialize kn admin plugin #2

Closed zhanggbj closed 4 years ago

zhanggbj commented 4 years ago

Hi All,

This kn-admin plugin is designed to help administrators or operators better manage a Knative platform installation with kn CLI.

This is an initial commit. More details in the proposal: https://docs.google.com/document/d/1oD5P4WQZs39GIr_4ezcURgRCuUx8dXrcCi3N7GKmp5c/edit#

Thanks!

knative-prow-robot commented 4 years ago

Welcome @zhanggbj! It looks like this is your first PR to knative/client-contrib 🎉

knative-prow-robot commented 4 years ago

Hi @zhanggbj. 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.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
zhanggbj commented 4 years ago

@rhuss Don't worry, take your time:-)

rhuss commented 4 years ago

@zhanggbj sorry, I couldn't make it before my PTO (and I'm heading now off to the Alpes with little connectivity). I will be back the first week in March and get your PRs merged then is on the top of my priority list.

Apologies, and have a nice week !

maximilien commented 4 years ago

/ok-to-test

maximilien commented 4 years ago

Hi, @zhanggbj, so all the tests are failing :(

I’ll leave some comments as well. But please fix this ^^^ first. Thanks.

knative-prow-robot commented 4 years ago

@zhanggbj: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
pull-knative-client-contrib-go-coverage ee9b4cc769655b6cd9616452c958e84ceb7c28ea link /test pull-knative-client-contrib-go-coverage

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
zhanggbj commented 4 years ago

@maximilien Thank you for you review!

I've address your most of your comments and left one comment (add version in next PR), would you please help to take a review, thx!

I check the test failure, looks like it doesn't relate do plugin code, it cannot find the proper test file to execute. For e.g.

+ ./test/presubmit-tests.sh --integration-tests
./test/presubmit-tests.sh: line 37: ./test/../vendor/knative.dev/test-infra/scripts/presubmit-tests.sh: No such file or directory
./test/presubmit-tests.sh: line 40: main: command not found
+ EXIT_VALUE=127
+ set +o xtrace
maximilien commented 4 years ago

Hi, @zhanggbj I think you need to add the test files in your PR. Doing that too for my PR #4.

I guess which ever gets merged first will have these files set and we can resolve conflicts in subsequent PRs.

Please do that and get all green and I will do a final pass. Thanks.

rhuss commented 4 years ago

/retest

rhuss commented 4 years ago

The following users are mentioned in OWNERS file(s) but are not members of the knative org.

Once all users have been added as members of the org, you can trigger verification by writing /verify-owners in a comment.

  • zhanggbj

    • OWNERS

@zhanggbj I asked the Knative admins to add you to the GitHub orga

zhanggbj commented 4 years ago

Hi @rhuss @maximilien, I've restructured kn-admin to align with the hello plugin, including folders/vendors/Reame/LICENSE/etc. would you please help to take a review? Thanks!

This is an initial PR for kn-admin and I still have a lot of TODOs in my list. I'll raise new PRs for review later as this already got a little bit hard to review:-)

rhuss commented 4 years ago

@zhanggbj btw, I would be happy to already commit the PR and then we can continue to work on it later.

zhanggbj commented 4 years ago

@rhuss I've addressed all your comments and would you please help to take a look? Thanks! And this is an initial PR, i think there're still a lot of todos for new subcommand and for existed domain and registry CRUD actions, and we can discuss later after this PR is merged and easy for collaboration.

knative-prow-robot commented 4 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhuss, zhanggbj

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: - ~~[OWNERS](https://github.com/knative/client-contrib/blob/master/OWNERS)~~ [rhuss] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment