-
Notifications
You must be signed in to change notification settings - Fork 391
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
fix: Get CI running again #1035
Conversation
* Update actions/checkout to v3 * Update actions/setup-python to v4 * Update actions/cache to v3
* Run 'pre-commit autoupdate'
* Add a lint job that is seperate from the testing as this is independent of the rest of the tests and only needs to be run once rather than multiple times. * Remove the linting with flake8 from the test jobs and make them require the 'lint' job to have passed to run. This has the same effect but avoids the possibility of dependency conflicts.
@mwouts this is ready for review. |
Wow thank you so much @matthewfeickert , this is very helpful! I will have a look at this asap. |
This looks really great! Thanks again. I see in your comment above that more cleaning could be done, that could be a great idea, feel free to tell us more about that! |
Sure. Some of this is just matters of preference, and obviously whatever works best for you is what you should do, but I've found it easier to maintain GitHub Actions workflows by separating them out into multiple workflows rather than having them all exist in one, like you normally would do for Travis CI, GitLab pipelines, etc. So for example, the jupytext/.github/workflows/continuous-integration.yml Lines 58 to 85 in e097422
job doesn't need to live in your CI workflow, but could exist as a separate Also, unless I'm mistaken about its purpose, the jupytext/.github/workflows/continuous-integration.yml Lines 11 to 19 in e097422
can be replaced with GitHub Actions concurrency control (c.f. https://docs.github.com/en/actions/using-jobs/using-concurrency). i.e. something like concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true These are just some thoughts, but more discussion would probably be better done in an Issue. I'll make one from this comment. |
I saw that
jupytext
's CI was broken so thought I'd try to help fix it. There's lots of cleaning that could be done on the CI, but this just gets it running again.This updates all the GitHub Actions to their latest releases and updates and applies the pre-commit hooks so those pass as well. It additionally fixes the dependency conflicts happening by trying to install the linting dependencies for older Pythons by just having the lint job be separate as it only needs to run once, not for every Python being tested.
I also threw in the use of
python -m pip
pattern over justpip
even though it isn't needed to try to spread this best practice to others who might be looking at jupytext's CI. Feel free to ask for that to be removed if you aren't a fan.