-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Override with env variables the cloud config #1583
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1583 +/- ##
==========================================
- Coverage 77.13% 77.09% -0.04%
==========================================
Files 162 162
Lines 13263 13270 +7
==========================================
+ Hits 10230 10231 +1
- Misses 2512 2518 +6
Partials 521 521
Continue to review full report at Codecov.
|
@mstoykov Could you rebase this on |
This will have to wait for #1599 as it likely will have conflicts either way |
This is how it should've been but ... because of the way this is configurable ... it is somewhat complicated ... The alternative to this (at this point) is to have the output know how to merge the env variables back ... which seems even worse
b3b8aaa
to
48c64ef
Compare
Codecov Report
@@ Coverage Diff @@
## master #1583 +/- ##
==========================================
+ Coverage 72.10% 72.22% +0.12%
==========================================
Files 167 175 +8
Lines 12882 13449 +567
==========================================
+ Hits 9288 9714 +426
- Misses 3031 3127 +96
- Partials 563 608 +45
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Given how fragile this code has been in the past and that with #1155 we will write completely new version of this handling I am closing this is issue in favor it just getting fixed then. This basically fixed if you have configured a cloud option through the script, to be able to overwrite it from outside the script. Which is likely is a very rare use case |
This is how it should've been but ... because of the way this is
configurable ... it is somewhat complicated ...
The alternative to this (at this point) is to have the output know how
to merge the env variables back ... which seems even worse