Overhaul crossplane --help output.
#4843
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This affects the order they appear when help is rendered. Sorting alphabetically helps folks scanning for an action (e.g. "build") or flag find it quickly in the output. Signed-off-by: Nic Cope <nicc@rk0n.org>
This touches only the help:"" struct tags and the Help methods that add more detailed help for commands. I'm not touching argument or flag help for now. In general I've tried to keep these as lightweight as possible. My thinking is that it's easier to update the docs than to update these help strings, so we should bias for most information living in the docs. Signed-off-by: Nic Cope <nicc@rk0n.org>
In particular I've tried to use placeholder values to communicate whether something is a path, a name, etc. I've stuck to the Kong pattern of defaults being specific and lowercase (e.g. file="example.yaml") while placeholder values are more generic and uppercase (e.g. file=PATH). Signed-off-by: Nic Cope <nicc@rk0n.org>
negz
commented
Oct 19, 2023
negz
commented
Oct 19, 2023
negz
commented
Oct 19, 2023
negz
commented
Oct 19, 2023
negz
commented
Oct 19, 2023
negz
commented
Oct 19, 2023
negz
commented
Oct 19, 2023
Otherwise we try to use LIMIT as the default value, and LIMIT is not an integer. Signed-off-by: Nic Cope <nicc@rk0n.org>
negz
commented
Oct 19, 2023
negz
commented
Oct 19, 2023
negz
commented
Oct 19, 2023
negz
commented
Oct 19, 2023
phisco
reviewed
Oct 19, 2023
| EmbedRuntimeImageName string `placeholder:"NAME" help:"The name of an OCI image to embed in the package as its runtime." xor:"runtime-image"` | ||
| EmbedRuntimeImageTarball string `placeholder:"PATH" type:"existingfile" help:"The tarball of an OCI image to embed in the package as its runtime." xor:"runtime-image"` | ||
| ExamplesRoot string `short:"e" type:"path" help:"A directory of example YAML files to include in the package." default:"./examples"` | ||
| Ignore []string `placeholder:"PATH" help:"Comma-separated paths, specified relative to --package-root, to exclude from the package."` |
There was a problem hiding this comment.
worth mentioning they are matched exactly and just setting a folder is not going to mean ignoring the whole content of that folder, or we could change the implementation to allow specifying folders directly.
There was a problem hiding this comment.
I lean toward changing the implementation, but let's do that later. 😬
cmd/crank/xpkg/build.go
Outdated
Comment on lines
125
to
126
| image (its "runtime"). Crossplane will then use the package as the provider | ||
| pod's image. |
There was a problem hiding this comment.
should we reject trying to build images with and embed image if the meta spec defines another image at spec.image?
There was a problem hiding this comment.
Probably a good idea, but let's tackle that separately.
plumbis
suggested changes
Oct 19, 2023
Signed-off-by: Nic Cope <nicc@rk0n.org>
Signed-off-by: Nic Cope <nicc@rk0n.org>
I'll add an example to cover this later. Signed-off-by: Nic Cope <nicc@rk0n.org>
Signed-off-by: Nic Cope <nicc@rk0n.org>
Frustratingly, there's no clear name for the URL-like thing that represents an OCI artifact in a registry. Sometimes it's a "name" or "image name". This is confusing because our kind: Provider resource also has a "name" - its metadata.name. I've biased for calling it "a package" and clarifying what it's made up of - i.e. a repo, tag, and registry. Signed-off-by: Nic Cope <nicc@rk0n.org>
Make them a lot more succinct, and focused on examples. Signed-off-by: Nic Cope <nicc@rk0n.org>
|
@phisco @plumbis PTAL. I didn't address everything exactly as requested, but I think I got the spirit of the desired changes. Namely:
|
phisco
approved these changes
Oct 20, 2023
Signed-off-by: Nic Cope <nicc@rk0n.org>
Trying to communicate that these affect a running Crossplane control plane (as opposed to other xpkg commands, that do not). Signed-off-by: Nic Cope <nicc@rk0n.org>
Signed-off-by: Nic Cope <nicc@rk0n.org>
This was referenced Nov 24, 2023
Open
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of your changes
This is a follow-on to #4832. Now that we have the overall command structured locked down we want to fine tune the help output.
I have:
make reviewableto ensure this PR is ready for review.Added or updated unit tests.Added or updated e2e tests.Linked a PR or a docs tracking issue to document this change.Addedbackport release-x.ylabels to auto-backport this PR.Need help with this checklist? See the cheat sheet.