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

Document the env directive and discuss environment handling in more detail. #248

Merged
merged 2 commits into from
Aug 25, 2024

Conversation

vext01
Copy link
Contributor

@vext01 vext01 commented Aug 5, 2024

I think this addresses the points raised in #247. Let me know what you think.

Fixes #247

@vext01
Copy link
Contributor Author

vext01 commented Aug 5, 2024

[As an aside, if you really want to start with the empty environment, you could use Popen(shell=False, ...), then the system shell could have no influence on the environment I think?]

@smarr
Copy link
Owner

smarr commented Aug 20, 2024

Sorry for the delay.

Looks great, though I just pushed two changes.
I added similar notes on env: for benchmark and executor and removed the Popen implementation detail: 18cabcc

Is that ok?

Thanks!

@smarr smarr force-pushed the improve-env-docs branch from 18cabcc to 98d1712 Compare August 25, 2024 15:30
Also remove the Popen implementation detail.

Signed-off-by: Stefan Marr <[email protected]>
@smarr smarr force-pushed the improve-env-docs branch from 98d1712 to 0c5c398 Compare August 25, 2024 15:34
Copy link
Owner

@smarr smarr left a comment

Choose a reason for hiding this comment

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

Will merge. Happy to improve at a later point.
Thanks!

@smarr smarr merged commit ae2c510 into smarr:master Aug 25, 2024
5 checks passed
@vext01 vext01 deleted the improve-env-docs branch August 27, 2024 09:30
@vext01
Copy link
Contributor Author

vext01 commented Aug 27, 2024

Thanks Stefan!

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.

rebench doesn't search the path correctly
2 participants