-
Notifications
You must be signed in to change notification settings - Fork 357
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: read scopes and contexts in env:get
, env:list
, and dev
#4839
Conversation
📊 Benchmark resultsComparing with 7317269 Package size: 228 MB⬇️ 0.00% decrease vs. 7317269
Legend
|
name: 'build settings', | ||
name: 'site settings', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@codebyuma I would very much appreciate a copy review in this PR for options outputted by the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's exciting to see this coming together! I haven't finished reviewing/testing but I need to run for an appointment, so I thought I'd submit the comments I have so far. I'll come back to this when I'm back
Co-authored-by: Uma Chandran <[email protected]>
…y/cli into feat/env-read-scopes-and-contexts
@netlify/netlify-dev this is ready for a code review now, if one of y'all can spare a moment! ❤️ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
THIS is an outstanding PR 👏 Very nicely documented and well tested! 💯
🐳
Why thank you @lukasholzer!! I realized a test was failing due to some capitalization of strings in logging 🤦 here's the change. Would you mind doing a re-review? |
Summary
This PR adds support for environment variable scopes and contexts for the
env:get
,env:list
, anddev
commands.env:get
andenv:list
support both the--scope
and--context
flags if the site has been configured with the new Environment Variables experience:tm:, aka Envelope. If the site is not configured with Envelope and either of those flags are passed, an error message will print. If this site is not configured with Envelope and neither of those flags are passed, it will behave as it normally has.A new Scope column has been added to
env:list
if the site is on Envelope:The
--context
flag fornetlify dev
is now supported, which will pull in values from Envelope while respecting the precedence of context values in netlify.toml.One caveat is that there will be a duplicate GET request sent to Envelope due to this change in netlify/config. Since we're adding context and scope options on the CLI level, I thought it would make sense to move the requests to happen in the CLI. Once all the calls to Envelope land in CLI, it should be safe to remove in netlify/config to remove the redundancy.
Fixes https://github.com/netlify/pillar-workflow/issues/755
Fixes https://github.com/netlify/pillar-workflow/issues/734
Test instructions
npm uninstall -g netlify-cli
npm link
env:get
,env:list
, anddev
a whirl. I recommend trying with a site opted-in to the new Env Vars experience in the UI (enable in Labs)For us to review and ship your PR efficiently, please perform the following steps:
passes our tests.
A picture of a cute animal (not mandatory, but encouraged)