-
-
Notifications
You must be signed in to change notification settings - Fork 163
Hard-coding coveralls secret #198
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
Hard-coding coveralls secret #198
Conversation
Putting the Coveralls secret in clear text (to be able to have coveralls info in forks) This means anyone can push information in Safe coveralls. We are ok with that "risk"
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.
Not sure why the coverage is not reported in the pull request, but it was successfully reported.
👍
Indeed. I would have expected a red flag here. I'm not sure what went wrong. |
f740037
to
207397e
Compare
Apparently the Github action expects a file in LCOV format that cannot be provided by PHPUnit :(
|
This reverts commit 207397e.
Ok. We are missing support from Github Actions from PHP-Coveralls package. PR is pending here: php-coveralls/php-coveralls#275 As soon as this is merged, I think we can try again and finally get code coverage is forks too. |
You should use github token instead I implemented it here https://github.com/webonyx/graphql-php/blob/599b03b59bf990e289337b1b79dc6aea31ea882e/.github/workflows/ci-build.yml#L159 (webonyx/graphql-php#605) |
Why not use https://codecov.io instead? With codecov, token-less uploads work fine on GitHub Actions. |
Not needed anymore since we use codecov, thanks for the tip @localheinz |
token is not issue with coveralls. Though the coveralls php repo is dead so covecov is definitely the better alternative. |
Putting the Coveralls secret in clear text (to be able to have coveralls info in forks)
This means anyone can push information in Safe coveralls. We are ok with that "risk"