-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Process] Allow inheriting env vars instead of replacing them #19053
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
Conversation
nicolas-grekas
commented
Jun 14, 2016
Q | A |
---|---|
Branch? | master |
Bug fix? | no |
New feature? | yes |
BC breaks? | no |
Deprecations? | no |
Tests pass? | yes |
Fixed tickets | #18796 |
License | MIT |
Doc PR | - |
|
||
if (!is_resource($this->process)) { | ||
echo $commandline; |
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.
should be removed
79963fe
to
5c6248b
Compare
Comments addressed, all green :) |
} | ||
$env = '\\' === DIRECTORY_SEPARATOR ? '(SET %s)&&' : 'export %s;'; | ||
foreach ($this->env as $k => $v) { | ||
$envline .= sprintf($env, ProcessUtils::escapeArgument("$k=$v")); |
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.
Don't we need one space after a value if it is followed by another key?
Never mind, you use one export
per environment variable.
* | ||
* @param bool $inheritEnv | ||
* | ||
* @return ProcessBuilder |
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.
@return is wrong
👍 |
1 similar comment
👍 |
Thank you @nicolas-grekas. |
…ng them (nicolas-grekas) This PR was merged into the 3.2-dev branch. Discussion ---------- [Process] Allow inheriting env vars instead of replacing them | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #18796 | License | MIT | Doc PR | - Commits ------- 524508c [Process] Allow merging env vars instead of replacing them