Skip to content

chore: Add helper for uniform flags and env vars#588

Merged
f0ssel merged 5 commits into
mainfrom
f0ssel/env
Mar 28, 2022
Merged

chore: Add helper for uniform flags and env vars#588
f0ssel merged 5 commits into
mainfrom
f0ssel/env

Conversation

@f0ssel
Copy link
Copy Markdown
Member

@f0ssel f0ssel commented Mar 27, 2022

This is a small package dedicated to cleaning and standardizing flags on the cli. By doing so we gain:

  • Default values for flags
  • Enforces pattern of matching flags and env vars (and requiring env vars use flags so we have better tests)
  • Generates clean usage docs for flags that have env var fallbacks & defaults

@f0ssel f0ssel force-pushed the f0ssel/env branch 2 times, most recently from da08c31 to b949d1a Compare March 28, 2022 00:00
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 28, 2022

Codecov Report

Merging #588 (13c71df) into main (b33dec9) will decrease coverage by 0.25%.
The diff coverage is 96.87%.

@@            Coverage Diff             @@
##             main     #588      +/-   ##
==========================================
- Coverage   63.68%   63.43%   -0.26%     
==========================================
  Files         195      196       +1     
  Lines       11313    11323      +10     
  Branches       85       85              
==========================================
- Hits         7205     7183      -22     
- Misses       3351     3378      +27     
- Partials      757      762       +5     
Flag Coverage Δ
unittest-go- 62.50% <96.87%> (-0.20%) ⬇️
unittest-go-macos-latest 58.39% <96.87%> (+0.10%) ⬆️
unittest-go-ubuntu-latest 61.33% <96.87%> (-0.01%) ⬇️
unittest-go-windows-2022 57.35% <96.87%> (-0.03%) ⬇️
unittest-js 63.32% <ø> (ø)
Impacted Files Coverage Δ
cli/workspaceagent.go 66.66% <66.66%> (ø)
cli/cliflag/cliflag.go 100.00% <100.00%> (ø)
cli/start.go 63.80% <100.00%> (-1.97%) ⬇️
provisionersdk/transport.go 74.46% <0.00%> (-6.39%) ⬇️
codersdk/provisionerdaemons.go 58.46% <0.00%> (-3.08%) ⬇️
coderd/provisionerdaemons.go 60.23% <0.00%> (-2.52%) ⬇️
peer/conn.go 79.18% <0.00%> (-1.53%) ⬇️
provisionerd/provisionerd.go 79.73% <0.00%> (-1.18%) ⬇️
peer/channel.go 83.62% <0.00%> (+0.58%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b33dec9...13c71df. Read the comment docs.

@f0ssel f0ssel force-pushed the f0ssel/env branch 5 times, most recently from b78809a to d5bf66c Compare March 28, 2022 03:36
@misskniss
Copy link
Copy Markdown

@kylecarbs can this be closed or does it still need to be done?

@kylecarbs
Copy link
Copy Markdown
Member

This isn't mandatory for alpha but should be something we tackle sooner rather than later.

Comment thread cli/cliflags/cliflags.go Outdated
Comment thread cli/cliflags/cliflags.go Outdated
Comment thread cli/cliflags/cliflags.go Outdated
Comment thread cli/cliflags/cliflags_test.go Outdated
Comment thread cli/start.go Outdated
Comment thread cli/cliflags/cliflags_test.go Outdated
Comment thread cli/cliflags/cliflags.go Outdated
Comment thread cli/cliflags/cliflags.go Outdated
Comment thread cli/cliflags/cliflags.go Outdated
Copy link
Copy Markdown
Member

@kylecarbs kylecarbs left a comment

Choose a reason for hiding this comment

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

Minor nits, but overall great addition!

Comment thread cli/cliflag/cliflag.go
Comment thread cli/cliflag/cliflag.go Outdated
@f0ssel f0ssel merged commit 01957da into main Mar 28, 2022
@f0ssel f0ssel deleted the f0ssel/env branch March 28, 2022 19:26
@misskniss misskniss added this to the V2 Beta milestone May 15, 2022
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.

3 participants