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

Rename 'CI' environment variable to 'COMPLEMENT_IN_DOCKER' or similar #60

Closed
anoadragon453 opened this issue Jan 18, 2021 · 3 comments · Fixed by #289
Closed

Rename 'CI' environment variable to 'COMPLEMENT_IN_DOCKER' or similar #60

anoadragon453 opened this issue Jan 18, 2021 · 3 comments · Fixed by #289

Comments

@anoadragon453
Copy link
Member

This grew out of a discussion here, where it was found that the CI environment variable is used by Complement to know whether it is running inside of a docker container already (and thus needs to talk to the host's docker socket) or is running on the host directly. It's assumed that if Complement is running inside of docker, CI=true.

@richvdh proposed to rename this variable to COMPLEMENT_IN_DOCKER (or something similar) instead, as it's more descriptive of what the variable actually specifies. Plus people won't be tempted to add CI-specific things outside of what's necessary (and thus making CI code split further from production code) 😇.

@kegsay
Copy link
Member

kegsay commented Jan 18, 2021

@richvdh
Copy link
Member

richvdh commented Jan 18, 2021

right, but the feature you're enabling here isn't "running under CI", it's "running under docker".

  • there's no reason why I shouldn't do this stuff locally as well as under CI.
  • there's nothing to say that if I'm running under CI, I'm inherently running inside a docker container. Sure that's the approach we take on our BK setup but other CI envs might solve the problem completely differently.
  • calling it CI tells me, as a complement user, absolutely nothing about what difference it makes, making it a scary black box.
  • calling it CI means that I might be tempted to make complement do other things differently under CI than in a normal run, which is generally a bad smell.

All in all: I think you chose ... poorly :)

@kegsay
Copy link
Member

kegsay commented Jan 18, 2021

I agree with your concerns and I'm not opposed to updating the name, but this was my thinking at the time.

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 a pull request may close this issue.

3 participants