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

Updated to use latest Sphinx version. #21

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

synchronizing
Copy link

@synchronizing synchronizing commented Dec 30, 2020

Unsure if this will break for some users. If not accepted, I recommend at least creating a new release with version 3 so that users may do:

uses: ammaraskar/sphinx-action@v3

@ammaraskar
Copy link
Owner

Hmm, since this is a breaking change I wonder if there's a way to add this as an option in the action, so anyone can override the sphinx version with whatever they prefer.

@synchronizing
Copy link
Author

synchronizing commented Dec 31, 2020

I've noticed that actions/upload-artifact can be used with @vX numbering at its end, like so:

    - name: Uploads the PDF build documentation
      uses: actions/upload-artifact@v2
      with:
        path: docs/build/latex/toolbox.pdf

By the looks of it, this correlates to the package releases. I'm unsure what package it would ultimately default to, but perhaps this is a solution.

@ammaraskar
Copy link
Owner

Right but a lot of people are pinned to @master right now, so even with the versioning it would break them.

@synchronizing
Copy link
Author

synchronizing commented Jan 1, 2021

Right. New release, perhaps?

@ax3l
Copy link

ax3l commented Sep 10, 2021

Since builds are broken anyway due to the Base image change for Debian, I would say we go head, start tagging versions and break once.
See for instance #32 #33.

ax3l added a commit to ax3l/sphinx-action that referenced this pull request Sep 10, 2021
- fix broken Debian Buster base image
- see ammaraskar#21
ax3l added a commit to ax3l/snowmass-compf2-accbeammodel.github.io that referenced this pull request Sep 10, 2021
Work-around for failing update. Lacks `latexmk` otherwise.
See
- ammaraskar/sphinx-action#33 (comment)
- ammaraskar/sphinx-action#21
ax3l added a commit to snowmass-compf2-accbeammodel/snowmass-compf2-accbeammodel.github.io that referenced this pull request Sep 10, 2021
* Snowmass: Restart & SM Day

Restart news & Snowmass Day.

* Fix Build (Buster now oldstable)

Work-around for failing update. Lacks `latexmk` otherwise.
See
- ammaraskar/sphinx-action#33 (comment)
- ammaraskar/sphinx-action#21
@vale981
Copy link

vale981 commented Dec 7, 2021

any news on this?

@synchronizing
Copy link
Author

synchronizing commented Dec 7, 2021

any news on this?

In my last few projects I've resorted to using my own branch. Unfortunately there seems to be preference of maintaining the older version (and therefore not breaking old packages that use this as a dependency), over updating this project (and breaking a few packages that might use it).

My take is simple. Create a release (v2) right now for Sphinx 2.4 (currently set), update Sphinx to its latest version at 4.3, and create a new release (v4) for Sphinx's version 4.3. Add a note the README with the specific error one would get in a repo which used 2.4, and simply tell them to add a @v2: actions/upload-artifact@v2.

@ferdnyc
Copy link

ferdnyc commented Dec 30, 2021

@ammaraskar

I'm trying to understand how updating the image is considered a "breaking change"?

For most users, the update won't break anything, they'll just get their docs built with a newer version, and probably benefit from the newer features provided by the upgraded version. That feels more like a positive than a negative, to me. There are a few users who might be negatively affected, but I suspect they're in the minority TBH. And if it causes problems, they can always switch to an older tag that works for them.

It seems especially odd worrying about the people who are using ammaraskar/sphinx-action@master, specifically. It would seem to me that carries an implicit understanding that the action will be updated over time, as it's explicitly requesting to track the latest release — not any specific version. Holding back your own master branch because it might impact those users who signed up to be impacted (though perhaps not knowingly, in all cases) just feels like misplaced concern, to be perfectly honest.

@ferdnyc
Copy link

ferdnyc commented Dec 30, 2021

All that being said, my idea in #40 for how to offer sphinxdoc/sphinx-latexpdf as an optional image may work for this, as well. Something in action.yml like...

inputs:
  sphinx4:
    description:
      Use a newer container image based on Sphinx 4.
    required: false
    default: false
runs:
  - using: 'docker'
    if: ${{ inputs.sphinx4 == false }}
    image: 'Dockerfile'
  - using: 'docker'
    if: ${{ inputs.sphinx4 == true }}
    image: 'Dockerfile.sphinx4'

would hopefully allow user selection of the base image.

(I guess I should go test that myself, now that I've mentioned it twice.)

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.

5 participants