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(cmd) use separate variables for Konnect flags #290

Merged
merged 1 commit into from
Mar 19, 2021

Conversation

rainest
Copy link
Contributor

@rainest rainest commented Mar 11, 2021

Use separate variables to store Konnect command flag values where Konnect commands share flags with non-Konnect commands.

Cobra will otherwise update the variable contents with whichever flags it processes last, effectively overriding the non-Konnect defaults with the Konnect defaults.

I don't think anything wants a different default value currently other than the state file, but I think we should define new variables for everything that's not actually being inherited from a parent command for consistency and safety in the event of future changes to one command set's defaults.

Fix #288

Use separate variables to store Konnect command flag values where
Konnect commands share flags with non-Konnect commands.

Cobra will otherwise update the variable contents with whichever flags
it processes last, effectively overriding the non-Konnect defaults with
the Konnect defaults.

Fix #288
@rainest rainest requested a review from a team March 11, 2021 23:34
@shaneutt shaneutt requested a review from hbagdi March 15, 2021 13:14
Copy link
Contributor

@mflendrich mflendrich left a comment

Choose a reason for hiding this comment

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

This problem is a perfect example of why we should avoid global state in decK.

Thanks @rainest for fixing this.

I recommend that we go further and isolate konnect commands into a subpackage.

@mflendrich mflendrich merged commit a190fd4 into main Mar 19, 2021
@mflendrich mflendrich deleted the fix/flag-collisions branch March 19, 2021 16:16
rainest pushed a commit that referenced this pull request Apr 21, 2021
Use separate variables to store Konnect command flag values where
Konnect commands share flags with non-Konnect commands.

Cobra will otherwise update the variable contents with whichever flags
it processes last, effectively overriding the non-Konnect defaults with
the Konnect defaults.

Fix #288
AntoineJac pushed a commit that referenced this pull request Jan 23, 2024
Use separate variables to store Konnect command flag values where
Konnect commands share flags with non-Konnect commands.

Cobra will otherwise update the variable contents with whichever flags
it processes last, effectively overriding the non-Konnect defaults with
the Konnect defaults.

Fix #288
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.

Deck always defaults to konnect.yaml instead of kong.yaml
2 participants