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

Allow environment variables to be passed to the compiler #28

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Allow environment variables to be passed to the compiler #28

wants to merge 2 commits into from

Conversation

olib963
Copy link

@olib963 olib963 commented Feb 15, 2020

Tackling this issue: #27

I have set the use_default_shell_env property to True to pass on any environment variables the user sets. I would like to suggest this as a better approach rather than setting the specific UTF8 values and trying to use that since it allows for other encodings and JVM config values clients may want.

I am happy to revisit a different approach if this is not desired, this feels less brittle to me though.

@olib963 olib963 marked this pull request as ready for review February 15, 2020 12:30
@olib963
Copy link
Author

olib963 commented Feb 15, 2020

Sorry I can go and fix the formatting if you like, my IntelliJ auto-formats on save and I didn't notice.

@olib963 olib963 changed the title Allow UTF8 compilation Allow environment variables to be passed to the compiler Feb 15, 2020
@SrodriguezO
Copy link
Contributor

Hey @olib963, thanks for opening this PR! Sorry for the slow response, things were really busy last week. I will take a look at this today.

@SrodriguezO
Copy link
Contributor

This looks great! Thanks for doing this. Would you be able to fix the formatting before we approve/merge? We should probably get some auto formatting going on this project, but I don't want to bloat the changes here.

@olib963
Copy link
Author

olib963 commented Mar 5, 2020

Sorry for the slow response. I've tried this out locally and for some reason when it's being called from this project, it will correctly use the environment. If you use it externally however it does not. I will do a little more digging and find out what's happening

@SrodriguezO
Copy link
Contributor

Interesting. Thanks for validating thoroughly :)

@JackTreble
Copy link

@SrodriguezO Is it possible to get this merged?
@olib963 👋 hope you are doing well :D

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