-
-
Notifications
You must be signed in to change notification settings - Fork 83
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
Translate validation error messages in the deserializer #1743
Conversation
✅ Deploy Preview for plone-restapi canceled.
|
@wesleybl thanks for creating this Pull Request and helping to improve Plone! TL;DR: Finish pushing changes, pass all other checks, then paste a comment:
To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass, but it takes 30-60 min. Other CI checks are usually much faster and the Plone Jenkins resources are limited, so when done pushing changes and all other checks pass either start all Jenkins PR jobs yourself, or simply add the comment above in this PR to start all the jobs automatically. Happy hacking! |
@jenkins-plone-org please run jobs |
@jenkins-plone-org please run jobs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's try using the latest version of everything for docs in requirements.txt
. Untested, but it follows the pattern in https://github.com/plone/training/blob/main/requirements.txt.
sphinxcontrib-devhelp==1.0.2 # newer versions require sphinx 5 | ||
sphinxcontrib-htmlhelp==2.0.1 # newer versions require sphinx 5 | ||
sphinxcontrib-qthelp==1.0.3 # newer versions require sphinx 5 | ||
sphinxcontrib-serializinghtml==1.1.5 # newer versions require sphinx 5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need all those sphinxcontrib-*help
s?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stevepiercy Yes, they are dependencies of Sphinx. They released new versions yesterday which are not compatible with Sphinx<5, but no longer declare that in their package metadata.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean that we don't build those types of outputs, so why add them at all? We use only HTML and linkcheck.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stevepiercy And I meant that as far as I can tell, we don't have a choice, because they are pulled in by sphinx.
Here's a requirements-docs.txt that worked for me.
Also requires a change to # "extra_navbar": """
# <p class="ploneorglink">
# <a href="https://plone.org">
# <img src="/_static/logo.svg" alt="plone.org" /> plone.org</a>
# </p>""", This should work on RTD where docs get published. It will not affect pulling in docs into the main documentation. |
@stevepiercy I didn't realize that the plone.restapi docs are still published separately at https://plonerestapi.readthedocs.io/ Do we really want that? (Doesn't it hurt SEO of the main docs?) |
Prior to Plone 6, there was no automatic documentation deploy process, so RTD stood in for it. I can't find the discussion about removing them from RTD. IMO it is a bad idea to publish them to RTD for SEO, branding, Marketing, and additional maintenance. Someone objected to their removal from RTD, the reason for which I cannot recall, and I had no interest in pushing it. |
@stevepiercy ok, I found the prior discussion about that: #1447 |
Fixes #1742