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

installer: clarify experimental config flag #8477

Merged
merged 1 commit into from
Mar 2, 2022

Conversation

Pothulapati
Copy link
Contributor

@Pothulapati Pothulapati commented Feb 28, 2022

Description

This PR updates the flag descriptions, and the render display
notes to be more explicit on what experimental config is.

This flag is required, to make sure users are opting in explicitely/
knowlingly to use the experimental config.

Signed-off-by: Tarun Pothulapati [email protected]

Related Issue(s)

Fixes #8107

How to test

render cmd should have the renamed naming, and messages

Release Notes

Renamed `danger-use-unsupported-config` flag in the installer to `use-experimental-config`

Documentation

/werft no-preview

@Pothulapati Pothulapati requested a review from a team February 28, 2022 07:14
@github-actions github-actions bot added the team: delivery Issue belongs to the self-hosted team label Feb 28, 2022
@csweichel
Copy link
Contributor

csweichel commented Feb 28, 2022

I'm a bit late to the party - the motivation for this switch is to have a clear (and deliberately inconvenient) hurdle to use experimental config. If we don't have a cost associated with this config, what distinguishes it from regular config other than some random bit "experimental" in the config?

@metcalfc's proposal was 💯 on point imho: just make the warning/message clearer and tell users what they have to do to use the experimental bit config.

Also, the assumption that "users have experimental config in there so they must know what they're doing" is flawed. Say you get config sent to you, don't really understand it in detail, then this flag will make (hopefully) make you stop and think for a second. Granted, with the Replicated layer on top that's less of a concern - but still valid. Imagine a new hire at Gitpod needing to use the installer and stumbling over this switch, you've just taught them there's "experimental config". Add some more detail to the flag description and error message, and they understand why it's there.

@Pothulapati
Copy link
Contributor Author

Say you get config sent to you, don't really understand it in detail, then this flag will make (hopefully) make you stop and think for a second. Granted, with the Replicated layer on top that's less of a concern - but still valid. Imagine a new hire at Gitpod needing to use the installer and stumbling over this switch, you've just taught them there's "experimental config". Add some more detail to the flag description and error message, and they understand why it's there.

Agree on the points specified here! Updated the PR to rename the flag, and have the display messages be clearer.

@Pothulapati Pothulapati changed the title installer: remove --use-experimental-config option installer: clarify experimental config flag Feb 28, 2022
Fixes #8107

This PR updates the flag descriptions, and the `render` display
notes to be more explicit on what experimental config is.

This flag is required, to make sure users are opting in explicitely/
knowlingly to use the experimental config.

Signed-off-by: Tarun Pothulapati <[email protected]>
@Pothulapati Pothulapati force-pushed the tar/instlaler-unsupported-flag branch from b95fd54 to 63458b8 Compare March 2, 2022 06:06
@mustard-mh
Copy link
Contributor

/hold
this PR block merge pool now, I leave hold here

@corneliusludmann
Copy link
Contributor

build is green now, next try

/unhold

@roboquat roboquat merged commit 61b50e7 into main Mar 2, 2022
@roboquat roboquat deleted the tar/instlaler-unsupported-flag branch March 2, 2022 10:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note size/XS team: delivery Issue belongs to the self-hosted team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[installer] Rename and print OR remove --danger-use-unsupported-config flag
6 participants