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

Add a function to set the value of the environment variable #1

Merged
merged 1 commit into from Dec 17, 2013

Conversation

Projects
None yet
3 participants
@eudoxia0
Contributor

eudoxia0 commented Dec 9, 2013

Users shouldn't have to care how the configuration variable is set internally, so I added a little functionality to set the value of a given env var.

Add a function to set the value of the environment variable bypassing…
… OSICAT. Users shouldn't have to care how the configuration variable is set internally.

fukamachi added a commit that referenced this pull request Dec 17, 2013

Merge pull request #1 from eudoxia0/master
Add a function to set the value of the environment variable

@fukamachi fukamachi merged commit 30275ff into fukamachi:master Dec 17, 2013

@avodonosov

This comment has been minimized.

Show comment
Hide comment
@avodonosov

avodonosov Feb 1, 2014

IMHO dependency on osicat is a huge overkill - you need a C compiler installed on your machine and cffi-grovel configured rightly to be able to invoke the C compiler. And that's only to load envy.

Moreover, the new functions introduced by this patch are not used by envy itself, because envy gets env variables using asdf::getenv, so the dependency on osicat is worthless.

avodonosov commented Feb 1, 2014

IMHO dependency on osicat is a huge overkill - you need a C compiler installed on your machine and cffi-grovel configured rightly to be able to invoke the C compiler. And that's only to load envy.

Moreover, the new functions introduced by this patch are not used by envy itself, because envy gets env variables using asdf::getenv, so the dependency on osicat is worthless.

@fukamachi

This comment has been minimized.

Show comment
Hide comment
@fukamachi

fukamachi Feb 1, 2014

Owner

IMHO dependency on osicat is a huge overkill - you need a C compiler installed on your machine and cffi-grovel configured rightly to be able to invoke the C compiler. And that's only to load envy.

Perhaps you're right. I'd like to remove the dependency.
But, are there any other choices for osicat:makunbound-environment-variable? Both of ASDF and UIOP have no function to do it.

Owner

fukamachi commented Feb 1, 2014

IMHO dependency on osicat is a huge overkill - you need a C compiler installed on your machine and cffi-grovel configured rightly to be able to invoke the C compiler. And that's only to load envy.

Perhaps you're right. I'd like to remove the dependency.
But, are there any other choices for osicat:makunbound-environment-variable? Both of ASDF and UIOP have no function to do it.

@eudoxia0

This comment has been minimized.

Show comment
Hide comment
@eudoxia0

eudoxia0 Feb 1, 2014

Contributor

I've been thinking about this too and I think it would be a good idea, especially given how osicat insists on recompiling everything every time it is loaded.

Contributor

eudoxia0 commented Feb 1, 2014

I've been thinking about this too and I think it would be a good idea, especially given how osicat insists on recompiling everything every time it is loaded.

@avodonosov

This comment has been minimized.

Show comment
Hide comment
@avodonosov

avodonosov Feb 1, 2014

@fukamachi, but osicat:makunbound-environment-variable is not used by envy.

avodonosov commented Feb 1, 2014

@fukamachi, but osicat:makunbound-environment-variable is not used by envy.

@avodonosov

This comment has been minimized.

Show comment
Hide comment
@avodonosov

avodonosov Feb 1, 2014

If there are users who want to remove environment variable, they can themselves load osical and use osicat:makunbound-environment-variable, no sense to export this function from envy under different name. In particular, if envy-test needs to remove env variable, envy test can depend on osicat, instead of hardcoding osicat dependency in envy.

avodonosov commented Feb 1, 2014

If there are users who want to remove environment variable, they can themselves load osical and use osicat:makunbound-environment-variable, no sense to export this function from envy under different name. In particular, if envy-test needs to remove env variable, envy test can depend on osicat, instead of hardcoding osicat dependency in envy.

@fukamachi

This comment has been minimized.

Show comment
Hide comment
@fukamachi

fukamachi Feb 2, 2014

Owner

@eudoxia0 Then, can I revert this pull request?

Owner

fukamachi commented Feb 2, 2014

@eudoxia0 Then, can I revert this pull request?

@eudoxia0

This comment has been minimized.

Show comment
Hide comment
@eudoxia0

eudoxia0 Feb 2, 2014

Contributor

@fukamachi I won't object, so you go ahead if you want.

Contributor

eudoxia0 commented Feb 2, 2014

@fukamachi I won't object, so you go ahead if you want.

fukamachi added a commit that referenced this pull request Feb 2, 2014

Revert "Merge pull request #1 from eudoxia0/master"
This reverts commit 30275ff, reversing
changes made to d62d215.
@fukamachi

This comment has been minimized.

Show comment
Hide comment
@fukamachi

fukamachi Feb 2, 2014

Owner

Okay, I reverted it.

Owner

fukamachi commented Feb 2, 2014

Okay, I reverted it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment