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.

Sign up for GitHub

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

Jump to bottom

Upload to coveralls.io only if 'push' on 'HazyResearch/fonduer' #392

Merged
merged 1 commit into from
Apr 11, 2020
Merged

Upload to coveralls.io only if 'push' on 'HazyResearch/fonduer' #392

merged 1 commit into from
Apr 11, 2020

Conversation

HiromuHota
Copy link
Contributor

@HiromuHota HiromuHota commented Apr 10, 2020 •
edited
Loading

This will close #391 .

@HiromuHota
Copy link
Contributor Author

Please see the step is skipped on a forked repository.
https://github.com/HiromuHota/fonduer/actions/runs/75405791.

lukehsiao
lukehsiao approved these changes Apr 10, 2020
View reviewed changes
Copy link
Contributor

@lukehsiao lukehsiao left a comment

Choose a reason for hiding this comment

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

Thanks!

@HiromuHota
Copy link
Contributor Author

HiromuHota commented Apr 10, 2020 •
edited
Loading

This PR fails.
https://github.com/HazyResearch/fonduer/actions/runs/75436780

coveralls.exception.CoverallsException: Not on TravisCI. You have to provide either repo_token in .coveralls.yml or set the COVERALLS_REPO_TOKEN env var.

  • The "Upload to coveralls.io" step runs because this PR is against 'HazyResearch/fonduer'
  • But, the step fails because a token for coveralls is not available.

@lukehsiao any idea?

@lukehsiao
Copy link
Contributor

Not sure. The configuration sets the env var, and the secret is in the repo:

image

This might be due to the secret not being passed correctly?

https://help.github.com/en/actions/configuring-and-managing-workflows/creating-and-storing-encrypted-secrets

@HiromuHota
Copy link
Contributor Author

HiromuHota commented Apr 10, 2020 •
edited
Loading

Oh, it is clearly stated in the image you attached:

Secrets are not passed to workflows that are triggered by a pull request from a fork.

@HiromuHota
Copy link
Contributor Author

Or maybe "Coveralls GitHub Action" works better.
https://github.com/marketplace/actions/coveralls-github-action

@HiromuHota HiromuHota marked this pull request as draft April 10, 2020 21:27
@HiromuHota
Copy link
Contributor Author

HiromuHota commented Apr 10, 2020 •
edited
Loading

"Coveralls GitHub Action" only supports LCOV file (lcov.info), while coverage.py, used by Fonduer, generates .coverage.
coverallsapp/github-action#30

For this reason, some people switched from coveralls to codecov.
executablebooks/MyST-NB#133

@HiromuHota HiromuHota changed the title Upload to coveralls.io only if github.repository == 'HazyResearch/fonduer' Upload to coveralls.io only if 'push' on 'HazyResearch/fonduer' Apr 10, 2020
@lukehsiao
Copy link
Contributor

Hmmm, I'm fine with switching to codecov, there is no particular reason we are using coveralls.

@senwu
Copy link
Collaborator

senwu commented Apr 10, 2020

I think codecov is easier solution here.

@HiromuHota HiromuHota marked this pull request as ready for review April 11, 2020 00:28
@HiromuHota
Copy link
Contributor Author

HiromuHota commented Apr 11, 2020 •
edited
Loading

Let's fix this for now. We can discuss if we should switch to codecov in a different issue/pr.

@lukehsiao lukehsiao merged commit c4f8d95 into HazyResearch:master Apr 11, 2020
lukehsiao pushed a commit that referenced this pull request Apr 12, 2020
@senwu
build(codecov): switch from coveralls to codecov
6349756 
Coveralls.io cannot be easily used by GitHub Actions because the secret token is not passed to PRs which are opened by third-party forks (#392). Furthermore, Coveralls.io's GitHub Action only supports LCOV files (coverallsapp/github-action#30). But, coverage.py generates .coverage files. 

Since they are roughly equivalent in functionality for our purpose, we switch from coveralls.io to CodeCov.
@AndreMiras
Copy link

Before doing the switch give that one a try, I've just built a coveralls action that works for pull requests https://github.com/marketplace/actions/coveralls-python

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lukehsiao lukehsiao lukehsiao approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

GitHub Actions fail on forked repository
4 participants
@HiromuHota @lukehsiao @senwu @AndreMiras