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

Move from Travis to GitHub Actions #470

Merged
merged 22 commits into from Dec 6, 2021
Merged

Move from Travis to GitHub Actions #470

merged 22 commits into from Dec 6, 2021

Conversation

JonathanHuot
Copy link
Contributor

As discussed in #460 and in #469, it seems simpler to use GitHub actions.

@JonathanHuot JonathanHuot changed the title Move from Travis to GitHub Actions WIP: Move from Travis to GitHub Actions Nov 24, 2021
@JonathanHuot
Copy link
Contributor Author

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.

@JonathanHuot
Copy link
Contributor Author

So with the AndreMiras/coveralls-python-action I get https://coveralls.io/builds/44520842.
Limitation: I though AndreMiras/coveralls-python-action#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
Copy link
Contributor

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?

Copy link
Contributor

@jtroussard jtroussard left a comment

Choose a reason for hiding this comment

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

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

.github/workflows/run-tests.yml Show resolved Hide resolved
.github/workflows/run-tests.yml Outdated Show resolved Hide resolved
.github/workflows/run-tests.yml Show resolved Hide resolved
@jtroussard
Copy link
Contributor

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

  • coverage-lcov
  • toml

@JonathanHuot
Copy link
Contributor Author

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 JonathanHuot changed the title WIP: Move from Travis to GitHub Actions Move from Travis to GitHub Actions Dec 6, 2021
@JonathanHuot
Copy link
Contributor Author

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!

Copy link
Contributor

@jtroussard jtroussard left a comment

Choose a reason for hiding this comment

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

lgtm

@jtroussard
Copy link
Contributor

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

@jtroussard jtroussard merged commit 2944df6 into master Dec 6, 2021
@jtroussard jtroussard added this to the 1.3.1 milestone Jan 2, 2022
@JonathanHuot JonathanHuot deleted the gh-actions branch February 25, 2024 21:26
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

2 participants