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

expose getenv in Sys module #78

Merged
merged 2 commits into from
Jul 18, 2019
Merged

expose getenv in Sys module #78

merged 2 commits into from
Jul 18, 2019

Conversation

Khady
Copy link
Contributor

@Khady Khady commented Jul 10, 2019

The module Sys0 contains getenv and is included in Sys. But the function is not exposed. Given that reading from the environment is a pretty common operation, I'm proposing the add getenv in sys.mli.

Signed-off-by: Louis Roché <louis@louisroche.net>
@bcc32
Copy link
Member

bcc32 commented Jul 15, 2019

@Khady, thanks for your contribution! I'd like to make a few changes to make this PR follow Base's conventions. Could you please enable Allow edits from maintainers on this PR?

@Khady
Copy link
Contributor Author

Khady commented Jul 16, 2019

C110F8DF-A965-4BA2-A48F-B9867BEDB324
It was already enabled. So you should be allowed to make edits.

Signed-off-by: Aaron L. Zeng <azeng@janestreet.com>
@bcc32
Copy link
Member

bcc32 commented Jul 16, 2019

Oh. I figured out I was pushing to the wrong ref. My bad!

@xclerc xclerc merged commit 42693c7 into janestreet:master Jul 18, 2019
@Khady Khady deleted the getenv branch July 18, 2019 09:04
@Khady
Copy link
Contributor Author

Khady commented Jul 18, 2019

thanks!

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.

3 participants