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

Update docs about preview from pull/merge requests #7823

Merged
merged 5 commits into from
Jan 26, 2021
Merged

Conversation

stsewd
Copy link
Member

@stsewd stsewd commented Jan 13, 2021

I can create a redirect after this is approved. I simplified the content from the guide.

@stsewd stsewd force-pushed the update-docs-prs branch 4 times, most recently from f77fe9e to 1fb7031 Compare January 13, 2021 14:33
Copy link
Member

@ericholscher ericholscher left a 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.


.. 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,
Copy link
Member

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?

Copy link
Member Author

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.
Copy link
Member

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 :/

Copy link
Member Author

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

# Don't index external version builds for now
if not version or version.type == EXTERNAL:
return

Copy link
Member Author

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.

Copy link
Member

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.

Copy link
Member

@ericholscher ericholscher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great 👍

@stsewd stsewd merged commit 0841038 into master Jan 26, 2021
@stsewd stsewd deleted the update-docs-prs branch January 26, 2021 20:53
@stsewd
Copy link
Member Author

stsewd commented Jan 26, 2021

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.

2 participants