Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix/feat: Add options for 'generator installer', remove unused -o/-f options from 'generator' base command help, and change some wording #596

Merged
merged 4 commits into from Sep 13, 2023

Conversation

ian-buse
Copy link
Contributor

@ian-buse ian-buse commented Aug 29, 2023

Made a few changes to the 'generator installer' command to make it easier to script with. The command now has --image-name and --image-tag options so users can specify these values when the kustomization YAML is generated, instead of manually editing the file after the fact. Some of the other options for this command were not utilizing the McMaster CLI default value feature, so those values have been added in-line to make it clearer to users what the defaults are if they do not provide them. I also reworded some of the option descriptions to be more unambiguous about what is expected to be in those folders.

BREAKING CHANGE:
Additionally, the Generator class no longer inherits from GeneratorBase, and GeneratorBase has been renamed to OutputBase. This is because the --out and --format options were shown under the generator command, but it seems they didn't actually do anything for that command. Instead, it seems those options are only used by the sub-commands themselves (docker, crds, rbac, etc) when the YAML/JSON is generated. So now, only the crd, rbac, docker, installer, and operator commands inherit from OutputBase. This will only be a breaking change for users who are unwittingly using the options on the generator command that go unused by the SDK.

If that shouldn't be a breaking change, some dummy options can be created and hidden under the generator command to prevent CLI parsing errors. However, this will result in unexpected behavior for new users if they accidentally place their -o/-f options on the wrong command (resulting in the contents being output to the console), which is somewhat similar to how it behaved before this PR.

@buehler
Copy link
Owner

buehler commented Sep 13, 2023

Hey @ian-buse
Thank you for the contribution!
Do you really think this is a break? If used correctly, the commands should work, even with this change right?

Because adding the flags to "generate" shouldn't do anything but printing the usage.

So, the correct usage would be as you suggest.

@ian-buse
Copy link
Contributor Author

Hi @buehler,

You’re welcome! Thanks for creating this SDK, it has really helped with development.

You're right, if the commands are used how they're supposed to, these changes shouldn’t break for anyone.

@buehler buehler merged commit 7812f42 into buehler:master Sep 13, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants