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

Document all environment variables used #187

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

gdm85
Copy link
Collaborator

@gdm85 gdm85 commented Nov 3, 2020

Document all environment variables which are directly read by goveralls.

This is important information to troubleshoot (think about running in a Docker container or integrating with a new CI).

Going forward we should add any new environment variables to the documentation.

@coveralls
Copy link

coveralls commented Nov 3, 2020

Coverage Status

Coverage increased (+0.04%) to 11.722% when pulling 9ac8430 on docs/env-variables into 51bf80d on master.


Some metadata used when submitting a Coveralls.io job can be specified via any of the mentioned environment variables:

* job ID: `COVERALLS_SERVICE_JOB_ID`, `TRAVIS_JOB_ID`, `CIRCLE_BUILD_NUM`, `APPVEYOR_JOB_ID`, `SEMAPHORE_BUILD_NUMBER`, `BUILD_NUMBER`, `BUILDKITE_BUILD_ID`, `DRONE_BUILD_NUMBER`, `BUILDKITE_BUILD_NUMBER`, `CI_BUILD_ID`, `GITHUB_RUN_ID`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it's extraneous to specify all environment variables that are maintained as part of CI integrations. Most just exist without the user needing to do something about them.

It should be sufficient to either have a couple of them standardized like git info/branch name, if we don't add them as command line options.

Though by design of the cli, seems adding them as options is more consistent.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disagree; you might be using Docker in one of these integrations (Jenkins for example) and then you need to know which environment variables are used by goveralls.

See for example how it is done for coveralls-python: https://github.com/coveralls-clients/coveralls-python/blob/6c8639cc22a5d9ef9f7e46fdb008fb3f53b99f26/docs/usage/tox.rst

Though by design of the cli, seems adding them as options is more consistent.

I don't see the current environment variables being replaced with CLI options any time soon, so documenting them is the minimum we can do. Some of them have tricky side-effects (see the GitHub Action integration or the Travis-CI case) which are also currently undocumented.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively, I could suggest documenting only a few "standard" environment variables, if the only objective is to pass them through to goveralls within a docker build.

BUILD_NUMBER, BRANCH_NAME and PULL_REQUEST_NUMBER can be documented to pass the individual CI env parameters to goveralls. The individual CI-specific environment variables differ drastically and, if required, may be documented in the CI's own section.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The individual CI-specific environment variables differ drastically and, if required, may be documented in the CI's own section.

I would rather go this way.

I could suggest documenting only a few "standard" environment variables, if the only objective is to pass them through to goveralls within a docker build.

When one is using Docker in Jenkins then the environment variables of Jenkins should be used; same goes for any other exotic combination which is in place, so I prefer to document them all for each C.I.

I will adjust this PR accordingly.

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

Successfully merging this pull request may close these issues.

None yet

3 participants