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

CI with GitHub Actions #329

Merged
merged 14 commits into from
May 9, 2021
Merged

CI with GitHub Actions #329

merged 14 commits into from
May 9, 2021

Conversation

Jhsmit
Copy link
Collaborator

@Jhsmit Jhsmit commented Mar 29, 2021

Moved CI to GitHub actions.

Currently functional:

  • pytest with coverage
  • Upload of coverage to Coveralls
  • Deploy on PyPi
  • Build docs

First 3 should work but cannot be tested from this PR

@Jhsmit Jhsmit marked this pull request as ready for review May 7, 2021 10:01
@Jhsmit
Copy link
Collaborator Author

Jhsmit commented May 7, 2021

@Jhsmit
Copy link
Collaborator Author

Jhsmit commented May 7, 2021

The current Travis CI procedure also builds the docs. Why is this needed? Arent they built by Readthedocs? Or is it to test if building the docs actually works?

@pckroon
Copy link
Collaborator

pckroon commented May 7, 2021

The current Travis CI procedure also builds the docs. Why is this needed? Arent they built by Readthedocs? Or is it to test if building the docs actually works?

Indeed. The CI should try to build the docs in nitpick mode to make sure it works, and RTD builds them without the nitpick flag so it's more likely to just work.

@Jhsmit
Copy link
Collaborator Author

Jhsmit commented May 7, 2021

Building docs was added and works.
Upload to PyPi done but currently fails due to missing tokens
Pytests run an pass for ubuntu/mac, fails on windows, but is fixed by #331

@tBuLi
Copy link
Owner

tBuLi commented May 8, 2021

First of all, excellent PR!

Alright, I have now created PYPI_API_TOKEN, TEST_PYPI_API_TOKEN , and COVERALLS_REPO_TOKEN, so at least the pypi ones should now work.

Why does the coveralls code use GITHUB_TOKEN? If I understood correctly, shouldn't that be COVERALLS_REPO_TOKEN?

@Jhsmit
Copy link
Collaborator Author

Jhsmit commented May 8, 2021

The coveralls GitHub action I've taken from the coveralls-pyton docs: https://coveralls-python.readthedocs.io/en/latest/usage/configuration.html#github-actions-support

Where it says that the COVERALLS_REPO_TOKEN is not needed.

I think this is ready to merge now, although I think #331 should be merged first such that when this merges the tests should pass and we can see if upload to coveralls works

@tBuLi tBuLi merged commit f44269b into tBuLi:master May 9, 2021
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 this pull request may close these issues.

None yet

3 participants