tBuLi / symfit

Symbolic Fitting; fitting as it should be.
http://symfit.readthedocs.org
MIT License
232 stars 17 forks source link

CI with GitHub Actions #329

Closed Jhsmit closed 3 years ago

Jhsmit commented 3 years ago

Moved CI to GitHub actions.

Currently functional:

First 3 should work but cannot be tested from this PR

Jhsmit commented 3 years ago

@tBuLi @pckroon For the PyPi deploy to work we would need a PyPI token to be added to GitHub's secrets: https://packaging.python.org/guides/publishing-package-distribution-releases-using-github-actions-ci-cd-workflows/#saving-credentials-on-github

See also: https://docs.github.com/en/actions/reference/encrypted-secrets

Jhsmit commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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