Skip to content
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

Fix passing environment variables to CmdAction #175

Closed
wants to merge 3 commits into from

Conversation

onnodb
Copy link
Contributor

@onnodb onnodb commented Apr 22, 2017

This should fix #171. It also includes a small test for passing environment variables to subprocesses.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.06%) to 99.729% when pulling 99a9b8c on onnodb:patch-171 into 1d00c7a on pydoit:master.

The "TestCmdAction.test_env" test contained a problem: the current
environment should be explicitly copied, and amended with the extra
environment variables to set, instead of passing in an otherwise empty
environment. This caused the tests to (rightly) fail on Windows CI
hosts.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.06%) to 99.729% when pulling 7f21004 on onnodb:patch-171 into 1d00c7a on pydoit:master.

@schettino72
Copy link
Member

thanks. please give me sometime to merge this as I will not be available next few days.

@onnodb
Copy link
Contributor Author

onnodb commented Apr 24, 2017

Sure, no rush. Thanks for your time and efforts in maintaining pydoit :-)

@@ -17,6 +23,12 @@
sys.stdout.write("out ouch")
sys.stderr.write("err output on failure")
sys.exit(11)
# check env
if len(sys.argv) == 3 and sys.argv[1]=='check' and sys.argv[2]=='env':
if os.environ.get('GELKIPWDUZLOVSXE') == '1':
Copy link
Member

Choose a reason for hiding this comment

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

so what GELKIPWDUZLOVSXE means? 😁

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just a random string really. Although I'm sure we can come up with a backronym 😁

@schettino72 schettino72 added this to the 0.31 milestone May 29, 2017
@schettino72
Copy link
Member

thanks. merged.

Are you on AUTHORS file? if not please add your name there.

@onnodb
Copy link
Contributor Author

onnodb commented May 29, 2017

Thanks, but such a tiny pull request is something I wouldn't like to call 'authoring' ;-) Once I make a more significant contribution, I'd be happy to add myself to the list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Passing environment variables to CmdAction is broken
3 participants