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(CLI): Automatically expand loaded env variables #9615
Conversation
Hello @d-asensio - could you please include motivation behind this PR proposal? I believe there is not corresponding issue created for that, correct? |
Hey @pgrzesik sure! I found some time to write an issue with the motivations of this. I referenced it in my first comment. Thank you! |
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.
Thanks a lot @d-asensio - it looks great overall and thanks so much for writing up a motivation in a linked issue. I think it's a good idea to allow such expansions, but it would also be good to document it e.g. here: https://github.com/serverless/serverless/blob/master/docs/environment-variables.md
Btw - it looks like there's a conflicting file - could you please look into that and rebase your branch?
f70f8f8
to
dd2eba6
Compare
Hey, @pgrzesik I rebased it onto master and added a comment with a little example in the docs. Thank you! |
dd2eba6
to
a158ad1
Compare
Hi @pgrzesik, is there something missing here? |
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.
Thanks a lot 🙇 Sorry for the delay in review - I missed the notification and then I was off for a few days. I have one minor suggestion related to docs - also, let's wait for opinion from @medikoo if we should introduce such extensions.
Codecov Report
@@ Coverage Diff @@
## master #9615 +/- ##
=======================================
Coverage 86.10% 86.10%
=======================================
Files 328 328
Lines 12590 12594 +4
=======================================
+ Hits 10840 10844 +4
Misses 1750 1750
Continue to review full report at Codecov.
|
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.
@d-asensio @pgrzesik it feels as great addition to me. It mimicks how things will work in a shell, which is a nice improvement.
@pgrzesik After adjustment to docs is made, feel free to merge
Thanks @medikoo 🙇 @d-asensio so the only thing to address is one comment about attributing what library is used for expansions and we're good to go 👍 |
…onment variables to the docs
f68ef3e
to
fd4b4ff
Compare
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.
Thank you @d-asensio, well done 👏
Closes: #9689