Skip to content

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

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Mar 28, 2022
Merged

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

merged 5 commits into from
Mar 28, 2022

Conversation

f0ssel
Copy link
Contributor

@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

codecov bot commented Mar 28, 2022 •
edited
Loading

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

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

@kylecarbs
Copy link
Member

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

kylecarbs
kylecarbs reviewed Mar 28, 2022
View reviewed changes
kylecarbs
kylecarbs approved these changes Mar 28, 2022
View reviewed changes
Copy link
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!

@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
Reviewers

@kylecarbs kylecarbs kylecarbs approved these changes

Assignees

@f0ssel f0ssel

Labels
None yet
Projects
None yet
Milestone
V2 Beta
Development

Successfully merging this pull request may close these issues.
3 participants
@f0ssel @misskniss @kylecarbs