Closed apcraig closed 1 year ago
This PR is trying to debug the new readthedocs errors.
Killing this, need to debug another way.
I wondered if it's actually a problem with the badge, since the documentation itself appeared to be fine. Is it not fine?
It's not the badge, that's the first thing I checked. I have created a new PR. Had to go thru the readthedocs logs carefully and do some googling. Seems to have been 2 minus signs that were "encoded" wrong. You can see the diff in the new PR, #432. But you can't tell there is a diff. I deleted the two minus signs and typed them back in. May have come from a copy and paste, hard to know.
If you look at the old documentation that had this, the html looks fine. The pdf was generated but these two minus signs were missing the pdf documentation.
Apparently, readthedocs changed behavior in the last day or two and it now fails the documentation if a pdf error occurs, wasn't doing that before.
The part I don't like is that I can create a PR to Icepack without the fixes and the github actions readthedocs check passes. So maybe that's just doing the html, not sure, but that could be a separate problem that we need to fix. I need to try to understand what github actions is doing and why it's not catching the pdf issue. May defer that, see how things go for a while.
I'm not sure how many people actually look at the pdf - we certainly haven't checked it carefully. Should we remove pdf as an output option and just provide the html? Maybe this conversation should be moved to an issue.
I think the pdfs are nice to have. Certainly easier to have a pdf on your laptop offline than to try to download the html directory or not have access when offline. My feeling at this point is to see how this pans out. If the pdf causes us constant problems, then lets create an issue and discuss. In some ways, there was an error in the documentation (for latex anyway) and now we'll be trapping those more consistently, so that's good, I think.
Icepack docs badge is fixed with merge of #432. Will do the same for CICE next.
PR checklist