search

requests / requests-oauthlib

OAuthlib support for Python-Requests!
https://requests-oauthlib.readthedocs.org/
ISC License
1.71k stars 421 forks source link

Move from Travis to GitHub Actions #470

Closed JonathanHuot closed 2 years ago

JonathanHuot commented 2 years ago

As discussed in https://github.com/requests/requests-oauthlib/issues/460 and in https://github.com/requests/requests-oauthlib/pull/469, it seems simpler to use GitHub actions.

JonathanHuot commented 2 years ago

I tested AndreMiras/coveralls-python-action, but found out important issue here https://github.com/AndreMiras/coveralls-python-action/issues/13

Coverage is passing and multiple github checks are added to the report, which is good. However, the data is pushed to coveralls as a "new coverage", i.e. we don't know if the coverage has increased or decreased compared to the master branch (base).

See as below image

I will move back to coverallsapp/github-action@master which was requiring a lcov format and not a standard format that coverage produce.

JonathanHuot commented 2 years ago

So with the AndreMiras/coveralls-python-action I get https://coveralls.io/builds/44520842. Limitation: I though https://github.com/AndreMiras/coveralls-python-action/issues/13 was a problem, but it seems I have the same behavior with the other one. I'm not sure at this stage if how can it be fixed.

When using the official coverallsapp/github-action I get https://coveralls.io/builds/44524845. Limitation: CI is more complex due to the format required lcov Limitation: python2.7 disabled Limitation: a change in the way coverage is calculated for property/setter compared to base.

jtroussard commented 2 years ago

I tested AndreMiras/coveralls-python-action, but found out important issue here AndreMiras/coveralls-python-action#13

Coverage is passing and multiple github checks are added to the report, which is good. However, the data is pushed to coveralls as a "new coverage", i.e. we don't know if the coverage has increased or decreased compared to the master branch (base).

See as below image

I will move back to coverallsapp/github-action@master which was requiring a lcov format and not a standard format that coverage produce.

Would the coverage comparison data not be shown if opening the "COVERAGE CHANGED" tab?

jtroussard commented 2 years ago

I think we can safetly add the following dependencies in requirements-test.txt:

JonathanHuot commented 2 years ago

Could we also inject a small doc/setup/getting started readme, specifically with the tox tool. I messed around with it for a while and could only get py2.7 stuff to work. I'm sure I'm screwing something up with the setup on my end.

UPDATE

So for potential documentation, I found that running tox -e <python-version-youre-working-with> simplifiled the local replication process. Also when running manually, be sure to install both requirements files (there are three, depends on which version of python you're running in venv) ... pip install -r requirements.txt && pip install -r requirements-test.txt AND pip install coveralls coverage-lcov toml

I have added a contributing guide, can you verify? Thanks!

JonathanHuot commented 2 years ago

I'm parking the coverage question as of now, as we will need to see it in action with a couple of PRs to see if we can live with it or not. Anyway it should not be a blocker to start reviewing some PRs.

The PR is ready to be merged if review is OK!

jtroussard commented 2 years ago

looks good - let's confirm Github actions are working after this merge