Migrate from Travis to GitHub Actions #2963
Conversation
Xarthisius
added
the
infrastructure
Xarthisius
force-pushed
the
gha
branch
12 times, most recently
from
47f3de3 to
0452eb6
Compare
Xarthisius
changed the title
Xarthisius
added
enhancement
|
I will review this as soon as I finish the release. I'm totally in support of the migration! |
| build: false | ||
|
|
||
| test_script: | ||
| - "echo 'Not testing anything. This is done on Travis'" |
There was a problem hiding this comment.
Deleting the appveyor.yml file will make this PR and all subsequent one fail until we deactivate appveyor completely. I would suggest delaying this to a future PR (otherwise I think we'd have to update all existing PRs to remove the appveyor.yml file).
There was a problem hiding this comment.
+1 to this. I think we should have dummy appveyor and travis files like what corentin already did with appveyor. What if we added a comment in both files that they'll be removed officially from master in ~3-6 months (and the date can be recorded in the file maybe) or something?
There was a problem hiding this comment.
There's always going to be a point when there are outstanding PRs and removing this file on master will force them to merge/rebase. I don't think setting a date will make a difference. As for appveyor I was planning on disabling it even before this goes in.
There was a problem hiding this comment.
Also I don't think anyone will need to remove appveyor.yml, unless they made some changes locally that were not on master.
There was a problem hiding this comment.
There's always going to be a point when there are outstanding PRs and removing this file on master will force them to merge/rebase.
Yes that's true. The reason I proposed this is because we've asked contributors to do this several times this year, which hasn't been the case in the past. I could see that be frustrating or even offputting.
| - name: Run Tests | ||
| env: | ||
| testsuite: ${{ matrix.tests-type }} | ||
| run: python -m nose -c nose_${testsuite}.cfg --traverse-namespace |
There was a problem hiding this comment.
Aren't we losing the coverage information?
There was a problem hiding this comment.
We will also probably need to modify the codecov webhook I think (?)
There was a problem hiding this comment.
Is the coverage working right now?
There was a problem hiding this comment.
Thank you so much for doing this @Xarthisius!!! I'm generally in approval of this, pending a few updates.
Do we want to add a Python 3.9 testing stage any time soon?
| libgeos-dev \ | ||
| libopenmpi-dev | ||
| ;; | ||
| osx|macOS) |
There was a problem hiding this comment.
does there need to be a windows step here too?
There was a problem hiding this comment.
nope, it's covered below.
| - name: Run Tests | ||
| env: | ||
| testsuite: ${{ matrix.tests-type }} | ||
| run: python -m nose -c nose_${testsuite}.cfg --traverse-namespace |
There was a problem hiding this comment.
We will also probably need to modify the codecov webhook I think (?)
|
This seems like a vast improvement. Is there anything left? It looks like coverage (maybe) and some comments? |
|
Just to sum up the current status:
@munkm @neutrinoceros @cphyc is there anything else for me to address? |
There was a problem hiding this comment.
@munkm @neutrinoceros @cphyc is there anything else for me to address?
Afaic this is now good to go !
It's good to go for me! |
|
🎉 thank you so much @Xarthisius !!!! |
It's my attempt to migrate our cloud testing infra to GitHub actions. I believe it has a parity with current Travis setup. I skipped one of the test runs (Py3.6 unittests non-minimal), because I think it was redundant. However, it can be trivially brought back. Comments are welcome at this point.