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.

Sign up for GitHub

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

Jump to bottom

Create separate environ variable helpers #1133

Merged
merged 1 commit into from
Apr 13, 2016
Merged

Create separate environ variable helpers #1133

merged 1 commit into from
Apr 13, 2016

Conversation

mglodack
Copy link
Contributor

@mglodack mglodack commented Feb 6, 2016

Currently, setProcessEnvironVar is exactly the same as setEnvironVar as noted here.

What has changed?
  • Mark setEnvironVar as obsolete and note to use setProcessEnvironVar.
What has been added?
  • Create individual environment variable setters that wrap the Environment.SetEnvironmentVariable method.

@mglodack mglodack force-pushed the update_environment_helper branch from b6f78cc to 5fe979a Compare February 6, 2016 06:04
@forki
Copy link
Member

forki commented Feb 6, 2016

I don't think we should make that obsolete. It's not harming that much, right?

@mglodack
Copy link
Contributor Author

mglodack commented Feb 7, 2016

@forki It's not harming. But for consistency, I thought that setEnvironVar would eventually be removed in a later version of FAKE.

Now all of the new exposed methods would include User, Machine, or Process.

@mglodack
Copy link
Contributor Author

What do you think @forki ?

@forki
Copy link
Member

forki commented Mar 31, 2016

I like the additions, but I don't really think we should make that other one obsolete

@mglodack mglodack force-pushed the update_environment_helper branch from 5fe979a to b342a85 Compare April 13, 2016 05:00
@mglodack
Copy link
Contributor Author

@forki I've removed the Obsolete

@forki forki merged commit ac84b5c into fsprojects:master Apr 13, 2016
@forki
Copy link
Member

forki commented Apr 13, 2016

thx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mglodack @forki