Env variables #16

Merged
merged 4 commits into from Mar 6, 2014

Conversation

Projects
None yet
2 participants
@lightsofapollo
Contributor

lightsofapollo commented Mar 5, 2014

See lightsofapollo/docker-worker@e9d9703 for a better view of the code changes without the vendor stuffs

@jonasfj

This comment has been minimized.

Show comment Hide comment
@jonasfj

jonasfj Mar 5, 2014

Member

If there is an appealing reason not to use a dictionary for environment variables, then at least add some docs to the README so we know how they should be given.
From what I can see it's

 env: ['MYVARIABLE=\'mystring\'', 'MYVARIABLE2=\'mystring2\'']

But doesn't this become problematic when my string contains things like " or ', I'm not a fan of double escaping... Especially not considering that you might have to escape an extra time in JSON :)

Member

jonasfj commented Mar 5, 2014

If there is an appealing reason not to use a dictionary for environment variables, then at least add some docs to the README so we know how they should be given.
From what I can see it's

 env: ['MYVARIABLE=\'mystring\'', 'MYVARIABLE2=\'mystring2\'']

But doesn't this become problematic when my string contains things like " or ', I'm not a fan of double escaping... Especially not considering that you might have to escape an extra time in JSON :)

@lightsofapollo

This comment has been minimized.

Show comment Hide comment
@lightsofapollo

lightsofapollo Mar 5, 2014

Contributor

@jonasfj You don't need to argue very hard to get objects to win... I greatly prefer these I just wish we didn't have to wrap docker... I will bug someone to see why they do this.

Contributor

lightsofapollo commented Mar 5, 2014

@jonasfj You don't need to argue very hard to get objects to win... I greatly prefer these I just wish we didn't have to wrap docker... I will bug someone to see why they do this.

lightsofapollo added a commit that referenced this pull request Mar 6, 2014

@lightsofapollo lightsofapollo merged commit 2086fd7 into taskcluster:master Mar 6, 2014

@lightsofapollo lightsofapollo deleted the lightsofapollo:env-variables branch Mar 6, 2014

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