-
-
Notifications
You must be signed in to change notification settings - Fork 136
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
base: master
Are you sure you want to change the base?
Conversation
|
||
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` |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
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.