Run sphinx-build as a module, ensures pipx compatibility #99
Conversation
There was a problem hiding this comment.
Running it as a module will make it easier to debug/track etc as well as faster as you mention w/o invoking the additional interpreter startup time too. And we're not using any benefits of parallelism or memory isolation so this looks good to me.
Knowing that the pipx approach sill works too is reasonable for us to keep, but I think that it would be better as a parallel test instead of just being inline here.
.github/workflows/test.yml
Outdated
| - name: Install apt dependencies | ||
| run: apt update && apt install -y doxygen graphviz | ||
|
|
||
| - name: Smoke test of pipx install |
There was a problem hiding this comment.
Can we drop this into a separate test to keep it clear that it's a separable test. It will run in parallel and get a separate report then too.
|
Oops, sorry about that. I'll fix it. |
|
I'm not sure what happened here. This fails CI when running on main: https://github.com/ros-infrastructure/rosdoc2/actions/runs/8726584290 |
This PR, a followup to PR #95, changes calling sphinx-build from a subprocess to a module. I claim this is useful regardless of any long-term plan of ros2 to recommend use of pipx, as spawning a new subprocess is probably more time consuming than simply running a python module, and we are not really using the subprocess for parallel processing in any way.
But yes this does ensure pipx compatibility, as demonstrated by the included extra testing.
Pragmatically though I'd like to land this now because it is top-of-mind at the moment, and if we revisit in a few months I'd have to figure this out again. I don't see any downside to this.