knative-extensions / kn-plugin-admin

Kn plugin for managing a Kubernetes based Knative installation.

Apache License 2.0

7 stars 18 forks source link

add autoscaling list command #29

Closed chaozbj closed 3 years ago

chaozbj commented 3 years ago

Add autoscaling list command.

chaozbj commented 3 years ago

@maximilien and @zhanggbj: I closed PR #17 because I can't make e2e test pass after I rebased it with three previous PRs. So I updated my master to latest and added autoscaling list back, and now the test is passed. Please help me to review. Thanks!

navidshaikh commented 3 years ago

@chaozbj : Could you try opening a fresh PR rebased onto current main branch and see if the github workflow update is picked up ? (I still see the UT are being run with go 1.15, while we downgraded that to 1.14)

chaozbj commented 3 years ago

@chaozbj : Could you try opening a fresh PR rebased onto current main branch and see if the github workflow update is picked up ? (I still see the UT are being run with go 1.15, while we downgraded that to 1.14)

ok, no problem

codecov[bot] commented 3 years ago

Codecov Report

Merging #29 (3b56b7f) into master (ab40d9d) will increase coverage by 1.13%. The diff coverage is 91.56%.

@@            Coverage Diff             @@
##           master      #29      +/-   ##
==========================================
+ Coverage   77.63%   78.76%   +1.13%     
==========================================
  Files          19       20       +1     
  Lines         921     1003      +82     
==========================================
+ Hits          715      790      +75     
- Misses        152      156       +4     
- Partials       54       57       +3

Impacted Files	Coverage Δ
pkg/command/autoscaling/autoscaling.go	`0.00% <0.00%> (ø)`
pkg/command/autoscaling/list.go	`92.59% <92.59%> (ø)`
pkg/command/autoscaling/update.go	`69.44% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ab40d9d...e7fce23. Read the comment docs.

chaozbj commented 3 years ago

@maximilien @navidshaikh @zhanggbj I addressed all comments and make UT and e2e passed, please help to review, thanks!

knative-prow-robot commented 3 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chaozbj, maximilien, 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-sandbox/kn-plugin-admin/blob/master/OWNERS)~~ [maximilien,zhanggbj] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment

zhanggbj commented 3 years ago

/lgtm /hold

Thanks @chaozbj for the efforts. Looks good to me. Hold for 1 day if there're any further comments from reviewers. If no, we can merge it.

maximilien commented 3 years ago

LGTM so will let others a chance to comment

zhanggbj commented 3 years ago

/unhold Merging it!