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

support for Sphinx v8.0 #376

Merged
merged 2 commits into from
Jul 30, 2024
Merged

support for Sphinx v8.0 #376

merged 2 commits into from
Jul 30, 2024

Conversation

2bndy5
Copy link
Collaborator

@2bndy5 2bndy5 commented Jul 28, 2024

using the 8.0.2 from pypi

Thanks to awareness in sphinx-doc/sphinx#12629

using the 8.0.0rc1 from pypi
Copy link
Collaborator Author

@2bndy5 2bndy5 left a comment

Choose a reason for hiding this comment

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

I suppose we could add the git master branch of sphinx to our CI. But that might be over-zealous given our sporadic maintainence.

Note

Dependabot does not support notifying us of pre-releases (that I'm aware of).

Copy link

codecov bot commented Jul 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.19%. Comparing base (6118569) to head (04e1ff2).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #376   +/-   ##
=======================================
  Coverage   67.19%   67.19%           
=======================================
  Files          68       68           
  Lines        8515     8515           
=======================================
  Hits         5722     5722           
  Misses       2793     2793           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jbms
Copy link
Owner

jbms commented Jul 29, 2024

I suppose we could add the git master branch of sphinx to our CI. But that might be over-zealous given our sporadic maintainence.

Note

Dependabot does not support notifying us of pre-releases (that I'm aware of).

Could be a good idea.

@2bndy5
Copy link
Collaborator Author

2bndy5 commented Jul 29, 2024

I'm ok with leaving this PR open until Sphinx v8 stable is released. If there are other pre-releases that need additional compliance, then I'll add it to this branch.

@2bndy5
Copy link
Collaborator Author

2bndy5 commented Jul 29, 2024

I've seen the master branch used in CI on the breathe project. It really just strained the sparse development efforts to maintain support for Sphinx' bleeding edge. We have enough monkey patching to worry about in Sphinx stable channel.

@jbms
Copy link
Owner

jbms commented Jul 29, 2024

Now that sphinx 8 is released it is probably time to submit this.

@2bndy5
Copy link
Collaborator Author

2bndy5 commented Jul 29, 2024

First, I should probably add sphinx v8 to CI tests job.

@jbms
Copy link
Owner

jbms commented Jul 29, 2024

Yeah. The number of combinations is getting quite high. Potentially we should run more than one sphinx version per job to save on setup cost.

@2bndy5
Copy link
Collaborator Author

2bndy5 commented Jul 29, 2024

Are you thinking the CI matrix for tests should only iterate over python version and node version and OS?

@2bndy5
Copy link
Collaborator Author

2bndy5 commented Jul 29, 2024

I think I could configure nox to skip certain sphinx versions if testing an unsupported python version.

@2bndy5 2bndy5 force-pushed the sphinx-v8.0 branch 3 times, most recently from 4485d0a to a20a6a4 Compare July 30, 2024 03:59
reduce combinations of CI test matrix

group nox sessions' output in CI logs

skip uploading a non-existent coverage HTML report in CI job
@2bndy5
Copy link
Collaborator Author

2bndy5 commented Jul 30, 2024

Ok I reduced the number of CI jobs by running tests per OS, python version and node.js version.

I also incorporated some GitHub workflow commands to group logs from certain nox sessions.
image
image

This should make it easier to read the CI logs when we need to find what went wrong in a certain nox session.

Note

There may be some discrepancies about which log() outputs (beginning and/or ending statements) are grouped because logger handlers acquire a stdout lock. Using nox.session.log() won't group the logs because GitHub expects the grouping commands to have nothing prefixed (the prefix nox > will negate log grouping). I tried to preempt this by using a separate Logger instance (that doesn't propagate to root handlers), but it doesn't seem to be consistent, although it is better than using print(..., flush=True).

It would be greatly beneficial (in CI runtime) to be able to cache the graphviz installation, but the amount of dependencies for that app make it near impossible to sustain.

@2bndy5 2bndy5 merged commit 9ec088e into main Jul 30, 2024
24 checks passed
@2bndy5 2bndy5 deleted the sphinx-v8.0 branch July 30, 2024 04:37
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.

2 participants