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

Add travis-ci style pull request builder #1340

Closed
alex opened this issue Jun 12, 2015 · 33 comments
Closed

Add travis-ci style pull request builder #1340

alex opened this issue Jun 12, 2015 · 33 comments
Labels
Accepted Accepted issue on our roadmap Feature New feature

Comments

@alex
Copy link
Contributor

alex commented Jun 12, 2015

It'd be amazing for helping review documentation if RTD has a travis-ci pull request builder. It could watch for PRs/pushes, build docs for them, and then link them in the build status.

/cc @reaperhulk

@reaperhulk
Copy link

We do this with a jenkins builder right now on pyca/cryptography but if we could get it straight through RTD that would be wonderful.

@agjohnson
Copy link
Contributor

This is on our radar, but we need to make some design decisions around how to handle arbitrary HTML uploads without allowing completely arbitrary HTML uploads.

Closing this as a duplicate of some bug I know exists and I'll find later :)

@ericholscher
Copy link
Member

This isn't an arbitrary HTML upload ticket, it's a request to auto-build branches from PR's of projects that we know about. It's definitely something we should be doing.

@ericholscher ericholscher reopened this Jun 12, 2015
@ericholscher ericholscher added Improvement Minor improvement to code Status: accepted and removed Status: invalid labels Jun 12, 2015
@agjohnson
Copy link
Contributor

The ticket description here seemed to describe building docs on Travis. Reopening this as we are describing a Github/Bitbucket pull request integration.

There are some design decisions that would be required here. Namely, how to handle pull requests from branches on forks given our currnet data model. We could easily support branches pushed to the repo we know about by expanding the webhook to handle PR open/update.

@agjohnson agjohnson added the Needed: design decision A core team decision is required label Jun 12, 2015
@alex
Copy link
Contributor Author

alex commented Jul 5, 2015

This ticket isn't about building docs on travis; it's about building docs in the style of travis. That is, on every push for a PR, build the docs and link them so people can browse.

@bukzor
Copy link

bukzor commented Feb 26, 2016

If we could set up webhooks to tell readthedocs to build docs for pull requests that touch documentation, that would be ideal.

Currently when a pull request comes in that touches documentation, we have two distinctly distasteful options:

  1. Eyeball the changes and merge it if it looks right, then try to remember to double-check rtfd when it builds the merged master.
  2. Tell the requestor to set up rtfd for their branch. Even users that are intimately familiar with sphinx/readthedocs would often balk at this request, since it would have to be re-done each time the pull request used a unique branch name.

@ericholscher
Copy link
Member

I agree this makes a lot of sense. We need to implement some kind of "temporary" version that would map to the PR, and clone from the remote repository sending the PR. Then we'd need to delete the version when the branch got merged, but I agree this would be quite useful.

@bukzor
Copy link

bukzor commented Feb 26, 2016

ericholscher: I think those can all be done based on data from the github webhooks, but I've no idea where the implementing code would live.I suppose it would be a new endpoint for the rtfd site?

Tarrasch added a commit to Tarrasch/luigi that referenced this issue Jul 13, 2016
I saw that the build was failing, hence this is my attempt to fix it,
altough I don't know how to test this before merging it.
See https://readthedocs.org/projects/luigi/builds/4189801/

Unfortunately I couldn't find a way to connect a webhook between GitHub
PRs and readthedocs: readthedocs/readthedocs.org#1340

The reason I believe a version bump would fix it is because I see
exactly the same error as in sphinx-doc/sphinx#2465 and readthedocs
seem to use 1.3.5, causing the pdf docs to be outdated.
Tarrasch added a commit to spotify/luigi that referenced this issue Jul 13, 2016
I saw that the build was failing, hence this is my attempt to fix it,
altough I don't know how to test this before merging it.
See https://readthedocs.org/projects/luigi/builds/4189801/

Unfortunately I couldn't find a way to connect a webhook between GitHub
PRs and readthedocs: readthedocs/readthedocs.org#1340

The reason I believe a version bump would fix it is because I see
exactly the same error as in sphinx-doc/sphinx#2465 and readthedocs
seem to use 1.3.5, causing the pdf docs to be outdated.
@agjohnson agjohnson added this to the PR Integration milestone Oct 19, 2016
@agjohnson
Copy link
Contributor

Blocked on #2465

@agjohnson agjohnson added the Status: blocked Issue is blocked on another issue label Oct 19, 2016
@stsewd
Copy link
Member

stsewd commented Jan 7, 2018

I don't know how the GitHub API works for this case, but I think this feature #872 could be used for this

@ssbarnea
Copy link

Any news on this? I do build sphinx docs with Travis on multiple projects but it seems that even if building docs with travis may succeed, it may not succeed on RTD, causing more trouble.

As I find much easier to build docs myself, fully integrated into CI pipeline, I am starting to wonder what are the real benefits of using RTD in this case, as it seems to be a PITA.

Did anyone managed to find a workaround that would prevent people from merging PRs that would break building RTD documentation?

@stsewd
Copy link
Member

stsewd commented Mar 18, 2018

@ssbarnea this is what you get from rtd for free 😉 https://docs.readthedocs.io/en/latest/features.html

I'm not sure how this feature would be integrated (rtd would have a lot of temporal docs), you can have something like this for checking your builds

https://github.com/rtfd/readthedocs.org/blob/e73b266558af84745dd1cfc9e9dec7aa3e091dde/tox.ini#L21-L24

And maybe using rstcheck (something like this #3624)

@thorade
Copy link

thorade commented Apr 10, 2018

I would like to use this in combination with protected branches and status checks, so that PRs can only be merged if they do NOT break the building process on RTD.
https://help.github.com/articles/about-protected-branches/
https://help.github.com/articles/about-required-status-checks/
https://developer.github.com/v3/repos/statuses/

@KengoTODA
Copy link
Contributor

to whom it may concern,

I created a GitHub Probot app that enables RTD build and share its document URL in PR. It may cover a part of your requirements, please check.

@ericholscher
Copy link
Member

Interestingly, travis does it using the git aliases also: https://travis-ci.org/rtfd/readthedocs.org/jobs/482572527#L418

@davidfischer davidfischer added Accepted Accepted issue on our roadmap and removed Needed: design decision A core team decision is required labels Apr 11, 2019
@davidfischer
Copy link
Contributor

This feature is accepted and we plan to do it. Our plan is to do this after all build artifacts are stored in blob storage (see #5549 for a start on that) because this change will significantly increase our storage requirements as RTD would be storing build output for every PR docs build indefinitely. This will become higher priority in the latter half of 2019.

@alex
Copy link
Contributor Author

alex commented Apr 11, 2019

That's wonderful to hear! If you ever want a beta tester, don't hesitate to holler!

jktjkt added a commit to jktjkt/oopt-gnpy that referenced this issue Jul 3, 2019
Since ReadTheDocs does not support [1] running as a CI check on GitHub
yet, let's make sure that we at least run sphinx. This would have caught
a recent docs breakage (see parent commit).

[1] readthedocs/readthedocs.org#1340
jktjkt added a commit to jktjkt/oopt-gnpy that referenced this issue Jul 3, 2019
Since ReadTheDocs does not support [1] running as a CI check on GitHub
yet, let's make sure that we at least run sphinx. This would have caught
a recent docs breakage (see parent commit).

[1] readthedocs/readthedocs.org#1340
@stsewd
Copy link
Member

stsewd commented Aug 19, 2019

We have this feature in beta https://blog.readthedocs.com/building-docs-for-pull-requests/

@ericholscher
Copy link
Member

Going to call this one closed, as it's now in the codebase. We will slowly roll it out, but it's done 👍

@jakirkham
Copy link
Contributor

What are the steps for setting this up for a repo currently?

@stsewd
Copy link
Member

stsewd commented Oct 16, 2019

@jakirkham you mean locally or on the site? On the site you need to request a beta access https://blog.readthedocs.com/building-docs-for-pull-requests/

For a local installation you need to enable this flag for your projects EXTERNAL_VERSION_BUILD

https://docs.readthedocs.io/en/stable/guides/feature-flags.html

You can do this from the admin of Features

@jakirkham
Copy link
Contributor

Thanks for the detailed answer. Was meaning the site. Didn't know if things had changed in the past few months.

@flying-sheep
Copy link
Contributor

Since I find this issue faster than the docs: https://docs.readthedocs.io/en/stable/guides/autobuild-docs-for-pull-requests.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accepted Accepted issue on our roadmap Feature New feature
Projects
None yet
Development

No branches or pull requests