search

ammaraskar / sphinx-action

Github action that builds docs using sphinx and places errors inline
Apache License 2.0
194 stars 116 forks source link

Change Dockerfile to use sphinxdoc/sphinx base image #12

Closed cmannix closed 4 years ago

cmannix commented 4 years ago

This PR changes the Dockerfile base image to sphinxdoc/sphinx: https://www.sphinx-doc.org/en/master/usage/installation.html#docker .

This installs a couple more packages than before (see: https://hub.docker.com/r/sphinxdoc/sphinx/dockerfile), but removes the need to install sphinx manually and so simplifies the Dockerfile a little, and reduces the 'build' step time by about a third (30s -> 20s - I've only tested this on a small example repo (not that the size should matter I suppose), and then not exhaustively).

Also bumps the sphinx version to 2.4.4 but can change that back if necessary.

Just wondered if this was possible, turns out it was, so thought I'd open a PR for it if you're interested.

jreklund commented 4 years ago

Hi, does that Docker images automatically look for the requirements.txt file (as you deleted it)? All projects using extensions would fail to build otherwise.

cmannix commented 4 years ago

Hi there, good question - I believe that will still work as this line: https://github.com/ammaraskar/sphinx-action/blob/8b4f60114d7fd1faeba1a712269168508d4750d2/sphinx_action/action.py#L102 pulls in the requirements.txt from a downstream project. The requirements.txt I deleted was only for this Docker image as I understand it.

You can see an example project with a requirements.txt using this change and successfully pulling in a theme from requirements.txt here: https://github.com/cmannix/sphinx-action-example/pull/2/checks?check_run_id=597483980

ammaraskar commented 4 years ago

Thank you for the PR @cmannix! Didn't realize sphinx had official docker images, this will serve as a much better base! :)