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

Migrate to GitHub Actions. #1410

Merged
merged 24 commits into from
Nov 28, 2020
Merged

Migrate to GitHub Actions. #1410

merged 24 commits into from
Nov 28, 2020

Conversation

jezdez
Copy link
Collaborator

@jezdez jezdez commented Nov 28, 2020

Travis CI has a new pricing model which places limits on open source.

Many projects are moving to GitHub Actions instead, including Jazzband projects:

This is based on jazzband/contextlib2#26.

TODO:

  • @jezdez to add JAZZBAND_RELEASE_KEY to the repo secrets.

@codecov
Copy link

codecov bot commented Nov 28, 2020

Codecov Report

Merging #1410 (b74c95b) into master (dfc863e) will decrease coverage by 0.26%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1410      +/-   ##
==========================================
- Coverage   88.13%   87.86%   -0.27%     
==========================================
  Files          29       29              
  Lines        1576     1574       -2     
  Branches      221      221              
==========================================
- Hits         1389     1383       -6     
- Misses        139      141       +2     
- Partials       48       50       +2     
Impacted Files Coverage Δ
debug_toolbar/panels/cache.py 83.58% <ø> (ø)
debug_toolbar/panels/history/panel.py 95.55% <100.00%> (-0.19%) ⬇️
debug_toolbar/panels/settings.py 87.50% <0.00%> (-12.50%) ⬇️
debug_toolbar/panels/sql/tracking.py 87.71% <0.00%> (-1.76%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update be55e23...b74c95b. Read the comment docs.

@jezdez
Copy link
Collaborator Author

jezdez commented Nov 28, 2020

So this is a bit more complicated GHA config since it tests three databases and has extra test environments to do linting, and a single environment to do Selenium testing (which is installde by default).

I'm a bit worried that this takes way too long for each Python version, but I couldn't figure out how to do this better (e.g. using more dimensions like database engine).

If any of the reviewers have an idea how to improve this, I'd appreciate it! Thank you :)

@hugovk
Copy link
Contributor

hugovk commented Nov 28, 2020

One idea is to factor things back out to the matrix. The earlier Travis config had more or less one job per Python x Django version. Maybe do that here, or for the database engines.

Have a look at this example, you can see an OS x Python matrix:

https://github.com/hugovk/pypistats/blob/7033d0adff90726f439b1b804485fac3fe92c6ec/.github/workflows/test.yml#L10-L12

You can also use include: to things not generated by the matrix. For example, something like this should give us 4x3 jobs (4 Python x 3 OS) + 1 extra (3.5 on Ubuntu):

      matrix:
        python-version: [3.6, 3.7, 3.8, 3.9]
        os: [ubuntu-latest, macos-latest, windows-latest]
        include:
          - { python-version: 3.5, os: ubuntu-latest }

(The includes in https://github.com/hugovk/pypistats/blob/7033d0adff90726f439b1b804485fac3fe92c6ec/.github/workflows/test.yml#L13-L18 are a bit different: because the OS matches something in the matrix, it adds a new variable for those.)

Similarly, it's possible to exclude things. This should give us 4x3 - 1 (skip 3.6 on macOS):

      matrix:
        python-version: [3.6, 3.7, 3.8, 3.9]
        os: [ubuntu-latest, macos-latest, windows-latest]
        exclude:
          - { python-version: 3.6, os: ubuntu-macos-latest }

Here's an exclusion example: https://github.com/hugovk/tinytext/blob/f1553124d2ab476e9d096ff521a61c9d57587d86/.github/workflows/test.yml#L22-L24

Would something like this help?

@jezdez
Copy link
Collaborator Author

jezdez commented Nov 28, 2020

@hugovk I've stolen some of techniques for this from @laymonage's django-jsonfield-backport and I think just specifying multiple jobs is the only way to cater to the mix of GHA-level factors and tox-level factors. Or at least I wasn't able to make this happen using tox-gh-actions. I can live with the slight code duplication. WDYT?

@hugovk
Copy link
Contributor

hugovk commented Nov 28, 2020

I think this is good, especially to get the migration done, and it can always be iterated on later.

@jezdez
Copy link
Collaborator Author

jezdez commented Nov 28, 2020

Thanks, agreed! 😌

@jezdez jezdez merged commit 621019c into master Nov 28, 2020
@jezdez jezdez deleted the gha branch November 28, 2020 19:27
@matthiask
Copy link
Member

Thanks, this is awesome!

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.

3 participants