Skip to content

Conversation

@joelgallant
Copy link
Contributor

@joelgallant joelgallant commented Jan 27, 2021

Closes #20.

I'm not entirely happy with the name environmentSourceNames, but environmentVariableName is already used for 'APP_CONFIG'.

@joelgallant joelgallant added the enhancement New feature or request label Jan 27, 2021
@joelgallant joelgallant requested a review from gregnr January 27, 2021 14:03
@joelgallant joelgallant self-assigned this Jan 27, 2021
@cypress
Copy link

cypress bot commented Jan 27, 2021



Test summary

9 0 0 0


Run details

Project App Config
Status Passed
Commit 17d1894
Started Jan 27, 2021 5:42 PM
Ended Jan 27, 2021 5:44 PM
Duration 01:33 💡
OS Linux Ubuntu - 18.04
Browser Electron 87

View run in Cypress Dashboard ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@codecov-io
Copy link

codecov-io commented Jan 27, 2021

Codecov Report

Merging #98 (17d1894) into master (a829ade) will increase coverage by 0.03%.
The diff coverage is 97.14%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #98      +/-   ##
==========================================
+ Coverage   89.03%   89.06%   +0.03%     
==========================================
  Files          21       21              
  Lines        1724     1738      +14     
  Branches      407      414       +7     
==========================================
+ Hits         1535     1548      +13     
- Misses        189      190       +1     
Impacted Files Coverage Δ
app-config/src/environment.ts 92.85% <90.90%> (-7.15%) ⬇️
app-config/src/config-source.ts 92.13% <100.00%> (+0.08%) ⬆️
app-config/src/config.ts 96.66% <100.00%> (+0.19%) ⬆️
app-config/src/extensions.ts 90.04% <100.00%> (ø)
app-config/src/meta.ts 94.73% <100.00%> (ø)

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 a829ade...17d1894. Read the comment docs.

Copy link
Contributor

@gregnr gregnr left a comment

Choose a reason for hiding this comment

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

This is awesome, thanks for implementing this so quickly!

I agree that naming this new variable is tough since we have many meanings of 'environment' in app-config. Other ideas that come to mind are something like environmentInputNames or environmentSourceNames since it describes the names of variables used for environment input/source.

@joelgallant joelgallant merged commit 3d53a24 into master Jan 27, 2021
@delete-merged-branch delete-merged-branch bot deleted the environment-meta-settings branch January 27, 2021 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Towards App Config CLI Parity w/ Node.js API

3 participants