Skip to content
This repository has been archived by the owner on Oct 10, 2023. It is now read-only.

Update generate-all-docs command logic to fetch installed plugins #3192

Merged
merged 1 commit into from
Sep 9, 2022
Merged

Update generate-all-docs command logic to fetch installed plugins #3192

merged 1 commit into from
Sep 9, 2022

Conversation

chandrareddyp
Copy link
Contributor

@chandrareddyp chandrareddyp commented Aug 24, 2022 •
edited
Loading

What this PR does / why we need it

The tanzu generate-all-docs command is broken because it's using the legacy API to fetch installed plugins and does not consider user input value for the output directory; this PR fixes below:

  1. tanzu generate-all-docs command logic updated to use context-aware plugin list API, and deprecate the legacy API
  2. tanzu generate-all-docs --docs-dir command logic updated to consider the user input value for the input flag '--docs-dir' docs output directory and generate docs to the given output directory
  3. update the error message if there is any error while reading the output directory

Which issue(s) this PR fixes

Fixes #1765

Describe testing done for PR

  • Manual testing is done for tanzu generate-all-docs to test docs for all installed plugins
  • Manual testing is done for tanzu generate-all-docs --docs-dir to test docs to be generated on the given output directory
  • Unit test cases are updated

Release note

`tanzu generate-all-docs` command implementation has been updated to use context-aware plugin list API and generate docs to given output folder

Additional information

Special notes for your reviewer

@chandrareddyp chandrareddyp requested a review from a team as a code owner August 24, 2022 22:33
@codecov
Copy link

codecov bot commented Aug 24, 2022 •
edited
Loading

Codecov Report

Merging #3192 (b098f7e) into main (9057a8e) will increase coverage by 0.70%.
The diff coverage is 37.73%.

@@            Coverage Diff             @@
##             main    #3192      +/-   ##
==========================================
+ Coverage   45.78%   46.49%   +0.70%     
==========================================
  Files         361      342      -19     
  Lines       37653    35301    -2352     
==========================================
- Hits        17240    16412     -828     
+ Misses      18836    17405    -1431     
+ Partials     1577     1484      -93
Impacted Files Coverage Δ
pkg/v1/cli/catalog.go 53.94% <ø> (ø)
pkg/v1/cli/command/core/doc.go 3.12% <4.76%> (-0.73%) ⬇️
pkg/v1/cli/pluginmanager/manager.go 64.81% <59.37%> (-0.31%) ⬇️
pkg/v1/tkg/tkgpackageclient/package_update.go 83.57% <0.00%> (-1.43%) ⬇️
pkg/v1/tkg/yamlprocessor/ytt.go
pkg/v1/tkg/kind/client.go
pkg/v1/tkg/utils/kubeconfig.go
pkg/v1/tkg/features/client.go
pkg/v1/tkg/utils/common.go
pkg/v1/tkg/yamlprocessor/ytt_definition_parser.go
... and 15 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@chandrareddyp chandrareddyp changed the title first cut code changes Update generate-all-docs command logic to fetch installed plugins Aug 26, 2022
@chandrareddyp chandrareddyp force-pushed the topic/cpamuluri/generate-all-docs-fix branch 3 times, most recently from 29d318f to 8cd1959 Compare August 30, 2022 14:59
marckhouzam
marckhouzam reviewed Aug 30, 2022
View reviewed changes
Copy link
Contributor

@marckhouzam marckhouzam left a comment

Choose a reason for hiding this comment

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

Very nice. LGTM.

Just a couple of nits and suggestions.

pkg/v1/cli/command/core/doc.go Outdated Show resolved Hide resolved
pkg/v1/cli/command/core/doc.go Show resolved Hide resolved
pkg/v1/cli/command/core/doc.go Show resolved Hide resolved
pkg/v1/cli/command/core/doc.go Show resolved Hide resolved
@chandrareddyp chandrareddyp force-pushed the topic/cpamuluri/generate-all-docs-fix branch from 6a649f5 to e2f7f94 Compare August 31, 2022 16:16
marckhouzam
marckhouzam reviewed Aug 31, 2022
View reviewed changes
Copy link
Contributor

@marckhouzam marckhouzam left a comment

Choose a reason for hiding this comment

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

Thanks for the changes.

This PR will have a minor conflict with @anujc25 large refactoring of #3201. I think adapting this PR will be easier than the other way around? If so, maybe we want to wait until #3201 is merged?

pkg/v1/cli/command/plugin/doc.go Outdated Show resolved Hide resolved
@chandrareddyp chandrareddyp force-pushed the topic/cpamuluri/generate-all-docs-fix branch from a45576c to fb30554 Compare August 31, 2022 16:42
marckhouzam
marckhouzam approved these changes Aug 31, 2022
View reviewed changes
Copy link
Contributor

@marckhouzam marckhouzam left a comment

Choose a reason for hiding this comment

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

LGTM

anujc25
anujc25 approved these changes Sep 1, 2022
View reviewed changes
Copy link
Contributor

@anujc25 anujc25 left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the fix.

@chandrareddyp chandrareddyp force-pushed the topic/cpamuluri/generate-all-docs-fix branch 2 times, most recently from 3792dc0 to 34bd886 Compare September 7, 2022 19:23
@chandrareddyp chandrareddyp mentioned this pull request Sep 7, 2022
Closed
10 tasks
anujc25
anujc25 reviewed Sep 7, 2022
View reviewed changes
cli/runtime/plugin/doc.go Outdated Show resolved Hide resolved
cli/runtime/plugin/doc.go Outdated Show resolved Hide resolved
@chandrareddyp chandrareddyp force-pushed the topic/cpamuluri/generate-all-docs-fix branch from 27c6954 to 7c4e4da Compare September 7, 2022 21:17
Update generate-all-docs command logic to generate output docs in cor…
b098f7e 
…rect directory

The tanzu generate-all-docs command is broken because it's using the legacy API to fetch installed plugins and does not consider user input value for the output directory; this PR fixes below:
1. 'tanzu generate-all-docs' command logic updated to use context-aware plugin list API, and deprecate the legacy API
2. 'tanzu generate-all-docs --docs-dir' command logic updated to consider the user input value for the input flag '--docs-dir' docs output directory and generate docs to the given output directory
3. update the error message if there is any error while reading the output directory
@chandrareddyp chandrareddyp force-pushed the topic/cpamuluri/generate-all-docs-fix branch from aafb53d to b098f7e Compare September 7, 2022 21:19
anujc25
anujc25 approved these changes Sep 7, 2022
View reviewed changes
Copy link
Contributor

@anujc25 anujc25 left a comment

Choose a reason for hiding this comment

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

LGTM

@anujc25 anujc25 added ok-to-merge PRs should be labelled with this before merging and removed ok-to-merge PRs should be labelled with this before merging labels Sep 7, 2022
@anujc25 anujc25 merged commit 82cab5a into main Sep 9, 2022
@anujc25 anujc25 deleted the topic/cpamuluri/generate-all-docs-fix branch September 9, 2022 17:57
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Reviewers

@marckhouzam marckhouzam marckhouzam approved these changes

@anujc25 anujc25 anujc25 approved these changes

@mpanchajanya mpanchajanya mpanchajanya approved these changes

Assignees
No one assigned
Labels
cla-not-required ok-to-merge PRs should be labelled with this before merging
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Non-context aware plugin listing is broken
5 participants
@chandrareddyp @marckhouzam @anujc25 @mpanchajanya @vmwclabot