-
-
Notifications
You must be signed in to change notification settings - Fork 79
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
Pin Sphinx version and make Netlify happy #1445
Conversation
Make Netlify happy
@ksuess 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! |
✅ Deploy Preview for plone-restapi ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@@ -2,4 +2,4 @@ | |||
PYTHON_VERSION = "3.8" | |||
|
|||
[build] | |||
ignore = "git diff --quiet $CACHED_COMMIT_REF $COMMIT_REF ./docs/" | |||
ignore = "git diff --quiet $BRANCH $HEAD -- ./docs/source/" |
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.
Comparing the changes on the branch. This has the effect, that the preview is done or at least kept, even when a further commit to the branch does not include any changes in /docs/source/.
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.
The Netlify log says fatal: bad revision 'pull/1445/head'
:
https://app.netlify.com/sites/plone-restapi/deploys/62a194ddac9c4d0008ab7e5a#L14
It still builds, so maybe it is OK? ¯\_(ツ)_/¯
Or perhaps it builds for every commit, and not only for changes in /docs/source
? You can test by pushing two trivial commits, one with a change in /docs/source
only, and one outside.
I also checked the Netlify logs, and it was building only for changes inside /docs/source/
up until this change. It's possible it was a one-time fluke.
While we're looking at this directory, can you delete /docs/requirements.txt
? It has been replaced by /requirements-docs.txt
.
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.
We need to test diffs with the new Netlify configuration. See comment.
Also remove obsolete file.
@@ -2,4 +2,4 @@ | |||
PYTHON_VERSION = "3.8" | |||
|
|||
[build] | |||
ignore = "git diff --quiet $CACHED_COMMIT_REF $COMMIT_REF ./docs/" | |||
ignore = "git diff --quiet $BRANCH $HEAD -- ./docs/source/" |
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.
The Netlify log says fatal: bad revision 'pull/1445/head'
:
https://app.netlify.com/sites/plone-restapi/deploys/62a194ddac9c4d0008ab7e5a#L14
It still builds, so maybe it is OK? ¯\_(ツ)_/¯
Or perhaps it builds for every commit, and not only for changes in /docs/source
? You can test by pushing two trivial commits, one with a change in /docs/source
only, and one outside.
I also checked the Netlify logs, and it was building only for changes inside /docs/source/
up until this change. It's possible it was a one-time fluke.
While we're looking at this directory, can you delete /docs/requirements.txt
? It has been replaced by /requirements-docs.txt
.
@stevepiercy @ksuess status? |
# Conflicts: # docs/source/conf.py # docs/source/endpoints/users.md # requirements-docs.txt
@tisto I merged master to this branch, fixed merge conflicts, and pushed. Let's see what Netlify builds. |
We switched from Netlify to Read the Docs in #1798, so this is outdated. |
No description provided.