ammaraskar / sphinx-action

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

Move master off of 2.4.4 #57

Open nicholasphair opened 7 months ago

nicholasphair commented 7 months ago

56 added support for different Sphinx versions through tags. master, however, still points to 2.4.4. That was purposeful. It was done to not immediately breaks builds and give people time to kick the tires on the tagging approach.

This issue is for tracking/coming up with a sane policy for moving the the master pointer to something more recent. Perhaps just a message in the logs encouraging folks to pin a specific version.

yinchi commented 4 weeks ago

Perhaps use an input to set the Docker image version? Default to latest if not set.

- name: Sphinx Build
  uses: ammaraskar/sphinx-action@master
    with:
      docs-folder: "docs"
      sphinx-tag: "7.4.7"

This way, the versioning of the Github Action is divorced from the version of Sphinx itself.

To get our new input into the Docker build, we could use Docker Container Action with build-arg. The Dockerfile would then start with:

ARG SPHINX_TAG=latest
FROM sphinxdoc/sphinx:$SPHINX_TAG

Our action.yml would contain something like:

runs:
  using: 'composite'
  steps:
    - uses: pl-strflt/docker-container-action@v1
      with:
        repository: ${{ steps.github.outputs.action_repository }}
        ref: ${{ steps.github.outputs.action_sha || steps.github.outputs.action_ref }}
        dockerfile: Dockerfile
        build-args: |
          SPHINX_TAG:${{ inputs.sphinx-tag }}
nicholasphair commented 3 weeks ago

Does that build an image each time the action is run? Would that not be slow? If not that could be a nice change.

Just to be clear, in case it is not, you can specify the sphinx version already:

- name: Sphinx Build
  uses: ammaraskar/sphinx-action@7.0.1
    with:
      docs-folder: "docs"

Good for end-users, but for maintainers of this project it requires effort to keep dockerfiles and tags in sync with the upstream sphinx project. Something that nobody really has time for and is why the versioning lags behind. That might be good motivation to consider #55. Though, that is certainly not my call.

In any case, the dev branch does have a script and a nightly workflow to try and automate this synchronization process. Maybe that can be merged @ammaraskar.

ammaraskar commented 3 weeks ago

I'm fine with it if you want merge dev into the main branch, I won't be able to review it at the moment.

yinchi commented 3 weeks ago

Unfortunately I couldn't make the suggestions in my previous post work. Will keep trying in my free time. For now I have my own fork with the version pinned to 7.4.7.

nicholasphair commented 3 weeks ago

Merging the dev branch would move master off 2.4.4 and I don't think anybody wants to deal with that right now. I did cherry-pick some of the tooling and automation though.

The cicd workflow should run nightly so we'll have to wait for a new sphinx release to see if it does its job. If it did, this repo should be kept in synch with sphinx going forward.

I also triggered the synch manually and there are now tagged versions of this action up until 8.0.2. So @yinchi I think you can use ammaraskar/sphinx-action@7.4.7.

That's about all the bandwidth I have to contribute at this time. Hope it helps.

Oh, and this issue should probably remain open since master is still on 2.4.4.

yinchi commented 3 weeks ago

Does that build an image each time the action is run? Would that not be slow? If not that could be a nice change.

@nicholasphair I don't think it's too slow, since our Dockerfile is just copying a few files over to the base image. The slowest parts appear to be handling requirements.txt if provided, and the actual build (both unavoidable).

The issue with my approach, I guess, is that it doesn't go well with the current tryrelease cronjob.