-
Notifications
You must be signed in to change notification settings - Fork 311
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
Finish transition from tox to Nox for current continuous integration checks and weekly tests #2694
Finish transition from tox to Nox for current continuous integration checks and weekly tests #2694
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2694 +/- ##
==========================================
+ Coverage 94.36% 95.15% +0.79%
==========================================
Files 107 107
Lines 9458 9458
Branches 2184 2184
==========================================
+ Hits 8925 9000 +75
+ Misses 345 276 -69
+ Partials 188 182 -6 ☔ View full report in Codecov by Sentry. |
To make sure that mypy is run consistently across platforms.
I updated the conditions when this workflow happens so that it is only done as a cron job for PlasmaPy/PlasmaPy@main, but will still run on PRs with the appropriate label, and by workflow dispatch.
…inst-dev-versions-of-dependencies
…inst-dev-versions-of-dependencies
…inst-dev-versions-of-dependencies
if: >- | ||
github.event_name == 'workflow_dispatch' || | ||
(github.event_name == 'pull_request' && contains(github.event.pull_request.labels.*.name, 'run weekly tests in CI')) || | ||
(github.event_name == 'schedule' && github.repository == 'PlasmaPy/PlasmaPy' && github.ref == 'refs/heads/main') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📝 This is saying that we should run tests...
- If we manually trigger it (via workflow dispatch)
- It's a pull request that has been labeled with "run weekly tests in CI"
- As a cron job if it's on the
main
branch ofPlasmaPy/PlasmaPy
.
…inst-dev-versions-of-dependencies
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m probably not the best person to review this since I wasn’t familiar with nox before this PR, but the motivation to move to it is clear and well documented. I see the benefits over tox so I support the move, and it looks to me like you did everything correctly here.
How big of a deal is the one failing weekly tests check? |
@sapols — Thank you kindly for the reviews! 🌱 It's much appreciated!
We'll need to address the astropy-dev error before our next release, so I'll create a separate issue to look into it. The error is already occurring in our weekly tests on |
This PR concludes the switchover from tox to Nox. I wanted to finish this ahead of the PlasmaPy Summer School so that participants won't need to learn how to work with tox (or more specifically,
tox.ini
).The changes in this PR include:
tox.ini
in its entirely, and there was much rejoicing. Using tox has overall been great, except for the INI-formatted configuration files! (We still havemypy.ini
...but I'm hoping that's only temporary.)requirements.txt
, as inspired by Requirements files need to be rethought #2647. It's not being used in CI anymore or by any Nox sessions, so we don't need it. Plus, IDEs are much more adept at getting requirements information frompyproject.toml
than they used to be.Some notes:
sudo apt install
step for pandoc and graphviz is specific to Ubuntu.mypy
to thetests
dependency set, primarily because then we won't have requirements information defined innoxfile.py
that we have to update.Remaining tasks
noxfile.py