vmware-tanzu / tanzu-cli

The Tanzu Core CLI project provides the core functionality of the Tanzu CLI. The CLI is based on a plugin architecture where CLI command functionality can be delivered through independently developed plugin binaries
Apache License 2.0
33 stars 21 forks source link

Prevent tanzu.md from being overwritten #752

Closed vuil closed 4 months ago

vuil commented 4 months ago

Since the plugin's generate-docs will be updated to produce a plugin-consistent toplevel tanzu.md, it is important that same file name produced by the CLI ifself be preserved.

The approach taken is to make a backup of the file before running the doc generation for each plugin, and restoring the file from backup.

What this PR does / why we need it

Which issue(s) this PR fixes

Fixes #

Describe testing done for PR

run generate-all-docs with plugins updated to produce its own tanzu.md verified that the tanzu.md generated the CLI is retained.

Release note

Fix generate-all-docs to account for installed plugins with mapped commands.

Additional information

Special notes for your reviewer

marckhouzam commented 4 months ago

By looking at this PR and at https://github.com/vmware-tanzu/tanzu-plugin-runtime/pull/183 as a global change, I realize that the root command of the CLI does not have a short description. This short description would be used in the generated markdown files. For example we see the missing short description in all core command files such as https://github.com/vmware-tanzu/tanzu-cli/blob/c9421d1a32e9644faf3dac125c1525372a84d00e/docs/cli/commands/tanzu_completion.md?plain=1#L70

What do you think of using this PR to add a short description Short: "The main Tanzu CLI" to the root command?

We could also use this PR to update the generated docs under docs/commands which are no longer up-to-date.

marckhouzam commented 4 months ago

One scenario that I noticed that isn't perfect but I don't think we need to fix it right away:

  1. I map a plugin under a CLI core command (I mapped builder to tanzu plugin builder)
  2. with the builder plugin using https://github.com/vmware-tanzu/tanzu-plugin-runtime/pull/183, the builder plugin will generate a "fake" tanzu_plugin.md doc file which will overwrite the real tanzu_plugin.md and we will loose the real content of that file

I wonder if we could generate the plugin markdown first and then the CLI core markdown so that we avoid CLI core commands risking being overwritten by a plugin? In fact, that would also prevent the tanzu.md file from being overwritten without needing to do a backup. What do you think?

vuil commented 4 months ago

By looking at this PR and at vmware-tanzu/tanzu-plugin-runtime#183 as a global change, I realize that the root command of the CLI does not have a short description. This short description would be used in the generated markdown files. For example we see the missing short description in all core command files such as

https://github.com/vmware-tanzu/tanzu-cli/blob/c9421d1a32e9644faf3dac125c1525372a84d00e/docs/cli/commands/tanzu_completion.md?plain=1#L70

What do you think of using this PR to add a short description Short: "The main Tanzu CLI" to the root command?

We could also use this PR to update the generated docs under docs/commands which are no longer up-to-date.

Thanks, yes of course, I even did it in vmware-tanzu/tanzu-plugin-runtime#183, and definitely meant to apply it here. I will go with "The Tanzu CLI", since on second thoughts the main doesn't say much.

vuil commented 4 months ago

One scenario that I noticed that isn't perfect but I don't think we need to fix it right away:

  1. I map a plugin under a CLI core command (I mapped builder to tanzu plugin builder)
  2. with the builder plugin using Update generate-docs to accounts for command mapping 2 tanzu-plugin-runtime#183, the builder plugin will generate a "fake" tanzu_plugin.md doc file which will overwrite the real tanzu_plugin.md and we will loose the real content of that file

I wonder if we could generate the plugin markdown first and then the CLI core markdown so that we avoid CLI core commands risking being overwritten by a plugin? In fact, that would also prevent the tanzu.md file from being overwritten without needing to do a backup. What do you think?

Thanks, I would need to think about it more on how to address this in a followup. Generating CLI core markdown later has the opposite issue of it overwriting the important tanzu_(pluginname).md that came from the plugin's generate-docs.

marckhouzam commented 4 months ago

Generating CLI core markdown later has the opposite issue of it overwriting the important tanzu_(pluginname).md that came from the plugin's generate-docs.

I didn’t try, but I assumed that generating the CLI markdown would only add files not remove any. Also, don’t think it would overwrite any valuable files generated by plug-ins before. Something to try out maybe when we have more time.