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

Set default environment variables #1045

Closed
wants to merge 1 commit into from
Closed

Conversation

watson
Copy link

@watson watson commented Nov 17, 2017

This allows code being run by the CI server to detect that it's running inside a CI server.

The CI environment variable is exposed by most major CI servers, but having a vendor specific environment variable (i.e. STRIDER) is also common practice.

For details see issue #892

This allows code being run by the CI server to detect that it's running
inside a CI server.

The `CI` environment variable is exposed by most major CI servers, but
having a vendor specific environment variable (i.e. `STRIDER`) is also
common practice.

For details see issue Strider-CD#892
@watson
Copy link
Author

watson commented Nov 17, 2017

This doesn't solve #892, but it's a low hanging fruit helping in a few of the scenarios where default environment variables are needed.

@xgalen
Copy link
Member

xgalen commented Nov 17, 2017

The tests are failing :S shit

@knownasilya
Copy link
Member

As far as I remember this will not set the env vars for the environment the code is ran in. I might be wrong, did you test this manually?

It looks like envs are set around here: https://github.com/Strider-CD/strider-simple-runner/blob/master/lib/index.js#L377

@watson
Copy link
Author

watson commented Nov 17, 2017

@knownasilya Ah good point. I'm not fully aware of how Strider will spawn the tests, but I'm 99% sure that this will not work as I intended.

In which case will the STRIDER_SSH_PUB and STRIDER_SSH_PRIV be set? I'm not sure exactly what config.envKeys is. Maybe I should just update this PR to set CI and STRIDER just outside that if-statement?

@knownasilya
Copy link
Member

I think that would be fine, also have a look at this addon, which changes the envs https://github.com/Strider-CD/strider-env/blob/master/config/config.js#L91

@watson
Copy link
Author

watson commented Nov 17, 2017

Oh, just realized that the file you linked to is in the strider-simple-runner repo and not this one. Is that the module that's responsible for actually executing the jobs?

@knownasilya
Copy link
Member

Exactly.

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