-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Update docs about preview from pull/merge requests #7823
Conversation
f77fe9e
to
1fb7031
Compare
1fb7031
to
f6cbec1
Compare
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.
This seems useful, but we need to clean up the wording here to make it less clunky.
We have links to both .org and GitHub, whereas we support this feature on both platforms, and on both VCS providers. We need to figure out a way to make this cleaner.
docs/pull-requests.rst
Outdated
|
||
.. note:: This feature is available only for :doc:`Sphinx projects </intro/getting-started-with-sphinx>`. | ||
|
||
- **Build Status Report:** When a build is triggered, a build pending notification is send with a link to the build log, |
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.
This should be above the Warning banner, it's a much bigger feature. I think we can probably just remove the warning banner section, since it's not really that useful to document?
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 think we should document it somewhere, as people some times expect this kind of fetatures to be on mkdocs projects as well. #7810
- Additional formats like PDF and Epub aren't build. | ||
- Searches will default to the default experience for your tool. | ||
This is a feature we plan to add, | ||
but don't want to overwhelm our search indexes used in production. |
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.
Is this true? I thought search just didn't work :/
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.
It should just work, as they are regular versions, we can enable it by removing this line
readthedocs.org/readthedocs/projects/tasks.py
Lines 1318 to 1320 in 74df71d
# Don't index external version builds for now | |
if not version or version.type == EXTERNAL: | |
return |
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.
now with the custom options for search I think this is useful, so users can test the results.
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.
Yea, it will just destroy our search indexes. I don't want to break prod search by indexing PR's, so we probably want a different cluster for it, if nothing else.
Co-authored-by: Eric Holscher <[email protected]>
7d05643
to
6e9d86a
Compare
6e9d86a
to
6f8f496
Compare
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.
This looks great 👍
Co-authored-by: Eric Holscher <[email protected]>
I can create a redirect after this is approved. I simplified the content from the guide.