Skip to content

src: update root and version structure and help text#865

Merged
knative-prow-robot merged 3 commits intoknative:mainfrom
lkingland:lkingland/cmd-root-and-version-update
Mar 2, 2022
Merged

src: update root and version structure and help text#865
knative-prow-robot merged 3 commits intoknative:mainfrom
lkingland:lkingland/cmd-root-and-version-update

Conversation

@lkingland
Copy link
Copy Markdown
Member

@lkingland lkingland commented Feb 28, 2022 •
edited
Loading

Changes

The root and version commands were far out of sync with more recent command structures. This minor PR updates them to the more recent format, updates their help text, and explicity references verbosity level in the version command.

  • 🧹 Update root command structure to match updated command structure
  • 🧹 Update root help text to match recent structure and deemphasize explicit Function name
  • 🧹 Update Version command to match updated command structure
  • 🧹 Adds an explicit check for verbosity in Version to fix what appears to be a bug
  • 🧹 Cleans up Version verbosity logic to be more clear about defaults

/kind cleanup

@knative-prow-robot knative-prow-robot added approved 🤖 PR has been approved by an approver from all required OWNERS files. size/L 🤖 PR changes 100-499 lines, ignoring generated files. labels Feb 28, 2022
@knative knative deleted a comment from knative-prow-robot Feb 28, 2022
@lkingland lkingland force-pushed the lkingland/cmd-root-and-version-update branch from 6d4f97c to 7baa609 Compare February 28, 2022 07:43
@codecov
Copy link
Copy Markdown

codecov bot commented Feb 28, 2022 •
edited
Loading

Codecov Report

Merging #865 (efd7dba) into main (3f30c91) will increase coverage by 0.34%.
The diff coverage is 81.31%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #865      +/-   ##
==========================================
+ Coverage   44.50%   44.84%   +0.34%     
==========================================
  Files          48       48              
  Lines        4743     4776      +33     
==========================================
+ Hits         2111     2142      +31     
- Misses       2334     2339       +5     
+ Partials      298      295       -3
Impacted Files Coverage Δ
cmd/version.go 66.66% <60.71%> (-33.34%) ⬇️
cmd/root.go 77.41% <90.47%> (+7.97%) ⬆️

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 3f30c91...efd7dba. Read the comment docs.

@lkingland lkingland changed the title chore: update root and version structure and help text src: update root and version structure and help text Feb 28, 2022
@lkingland lkingland mentioned this pull request Feb 28, 2022
Closed
zroubalik
zroubalik approved these changes Feb 28, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

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

/lgtm

/hold for others to review

@knative-prow-robot knative-prow-robot added the do-not-merge/hold 🤖 PR should not merge because someone has issued a /hold command. label Feb 28, 2022
@knative-prow-robot knative-prow-robot added the lgtm 🤖 PR is ready to be merged. label Feb 28, 2022
@lkingland lkingland mentioned this pull request Feb 28, 2022
Closed
lance
lance reviewed Feb 28, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@lance lance left a comment

Choose a reason for hiding this comment

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

/lgtm

@knative-prow-robot knative-prow-robot removed the lgtm 🤖 PR is ready to be merged. label Mar 1, 2022
@lkingland
Copy link
Copy Markdown
Member Author

lkingland commented Mar 1, 2022

@lance I added in a proper test which should address your comment regarding sampling the root commmand's output directly. Thanks for the feedback

@knative-prow-robot knative-prow-robot added the needs-rebase Cannot be merged due to conflicts with HEAD. label Mar 2, 2022
@lkingland lkingland force-pushed the lkingland/cmd-root-and-version-update branch from 60f9b93 to 673a6c4 Compare March 2, 2022 11:08
@knative-prow-robot knative-prow-robot removed the needs-rebase Cannot be merged due to conflicts with HEAD. label Mar 2, 2022
@lkingland lkingland force-pushed the lkingland/cmd-root-and-version-update branch from 673a6c4 to efd7dba Compare March 2, 2022 11:11
lance
lance reviewed Mar 2, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@lance lance left a comment

Choose a reason for hiding this comment

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

/lgtm

@knative-prow-robot knative-prow-robot added the lgtm 🤖 PR is ready to be merged. label Mar 2, 2022
@lance
Copy link
Copy Markdown
Contributor

lance commented Mar 2, 2022 •
edited
Loading

/hold to figure out e2e tests

@matejvasek
Copy link
Copy Markdown
Contributor

matejvasek commented Mar 2, 2022

I've just restarted the tests in case it's just a flake.

lance
lance approved these changes Mar 2, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@lance lance left a comment

Choose a reason for hiding this comment

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

/unhold

@knative-prow-robot knative-prow-robot removed the do-not-merge/hold 🤖 PR should not merge because someone has issued a /hold command. label Mar 2, 2022
@knative-prow-robot
Copy link
Copy Markdown

knative-prow-robot commented Mar 2, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lance, lkingland, zroubalik

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:
  • OWNERS [lance,lkingland,zroubalik]

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 merged commit 043a00d into knative:main Mar 2, 2022
@lkingland lkingland deleted the lkingland/cmd-root-and-version-update branch September 20, 2022 01:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@lance lance lance approved these changes

@zroubalik zroubalik zroubalik approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@lance lance

@zroubalik zroubalik

Labels

approved 🤖 PR has been approved by an approver from all required OWNERS files. lgtm 🤖 PR is ready to be merged. size/L 🤖 PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@lkingland @lance @matejvasek @knative-prow-robot @zroubalik