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

feat: add pretty printed default to validated flags #4078

Merged
merged 1 commit into from
May 25, 2023

Conversation

pmalek
Copy link
Member

@pmalek pmalek commented May 25, 2023

What this PR does / why we need it:

This adds a pretty printed default to ValidatedValueOpt[T] so that when one uses it as such:

flagSet.Var(flags.NewValidatedValue(&c.GatewayAPIControllerName, gatewayAPIControllerNameFromFlagValue, flags.WithDefault(string(gateway.GetControllerName()))), "gateway-api-controller-name", "The controller name to match on Gateway API resources.")

Instead of

--gateway-api-controller-name string             The controller name to match on Gateway API resources.

the end user can get

--gateway-api-controller-name string             The controller name to match on Gateway API resources. (default "konghq.com/kic-gateway-controller")

This will be useful for e.g. custom types like DNSStrategy in #4071.

Additionally this PR moves the types and functions related to validated flags to a separate package under internal/manager/flags.

@pmalek pmalek added this to the KIC v2.10.0 milestone May 25, 2023
@pmalek pmalek requested a review from a team as a code owner May 25, 2023 10:41
@pmalek pmalek self-assigned this May 25, 2023
@pmalek pmalek added the area/feature New feature or request label May 25, 2023
@pmalek pmalek force-pushed the add-pretty-printed-default-for-validated-cli-flags branch from 1e1d48d to 5f98b2d Compare May 25, 2023 10:58
@pmalek pmalek enabled auto-merge (squash) May 25, 2023 10:58
@codecov
Copy link

codecov bot commented May 25, 2023

Codecov Report

Patch coverage: 81.2% and project coverage change: -38.4 ⚠️

Comparison is base (ccafa7c) 59.9% compared to head (1e1d48d) 21.5%.

❗ Current head 1e1d48d differs from pull request most recent head 5f98b2d. Consider uploading reports for the commit 5f98b2d to get more accurate results

Additional details and impacted files
@@           Coverage Diff            @@
##            main   #4078      +/-   ##
========================================
- Coverage   59.9%   21.5%   -38.4%     
========================================
  Files        149     149              
  Lines      16520   16532      +12     
========================================
- Hits        9899    3567    -6332     
- Misses      5992   12491    +6499     
+ Partials     629     474     -155     
Impacted Files Coverage Δ
internal/manager/flags/validated.go 70.4% <75.0%> (ø)
internal/manager/config.go 94.5% <100.0%> (ø)

... and 116 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@pmalek pmalek merged commit e237dc0 into main May 25, 2023
24 checks passed
@pmalek pmalek deleted the add-pretty-printed-default-for-validated-cli-flags branch May 25, 2023 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/feature New feature or request size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants