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

Open
alex opened this Issue Jun 12, 2015 · 24 comments

Comments

Projects
None yet
@alex
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

This comment has been minimized.

Show comment
Hide comment
@reaperhulk

reaperhulk Jun 12, 2015

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.

reaperhulk commented Jun 12, 2015

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

This comment has been minimized.

Show comment
Hide comment
@agjohnson

agjohnson Jun 12, 2015

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

Contributor

agjohnson commented Jun 12, 2015

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

This comment has been minimized.

Show comment
Hide comment
@ericholscher

ericholscher Jun 12, 2015

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.

Member

ericholscher commented Jun 12, 2015

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.

@agjohnson

This comment has been minimized.

Show comment
Hide comment
@agjohnson

agjohnson Jun 12, 2015

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.

Contributor

agjohnson commented Jun 12, 2015

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.

@alex

This comment has been minimized.

Show comment
Hide comment
@alex

alex Jul 5, 2015

Contributor

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.

Contributor

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

This comment has been minimized.

Show comment
Hide comment
@bukzor

bukzor 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.

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

This comment has been minimized.

Show comment
Hide comment
@ericholscher

ericholscher Feb 26, 2016

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.

Member

ericholscher commented Feb 26, 2016

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

This comment has been minimized.

Show comment
Hide comment
@bukzor

bukzor 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?

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

docs: Set minimum versions for sphinx
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: rtfd/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

docs: Set minimum versions for sphinx (#1760)
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: rtfd/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

This comment has been minimized.

Show comment
Hide comment
@agjohnson

agjohnson Oct 19, 2016

Contributor

Blocked on #2465

Contributor

agjohnson commented Oct 19, 2016

Blocked on #2465

@stsewd

This comment has been minimized.

Show comment
Hide comment
@stsewd

stsewd Jan 7, 2018

Member

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

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

This comment has been minimized.

Show comment
Hide comment
@ssbarnea

ssbarnea Mar 18, 2018

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?

ssbarnea commented Mar 18, 2018

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

This comment has been minimized.

Show comment
Hide comment
@stsewd

stsewd Mar 18, 2018

Member

@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

readthedocs.org/tox.ini

Lines 21 to 24 in e73b266

[testenv:docs]
changedir = {toxinidir}/docs
commands =
sphinx-build -b html -d {envtmpdir}/doctrees . {envtmpdir}/html

And maybe using rstcheck (something like this #3624)

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

readthedocs.org/tox.ini

Lines 21 to 24 in e73b266

[testenv:docs]
changedir = {toxinidir}/docs
commands =
sphinx-build -b html -d {envtmpdir}/doctrees . {envtmpdir}/html

And maybe using rstcheck (something like this #3624)

@thorade

This comment has been minimized.

Show comment
Hide comment
@thorade

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

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/

@flying-sheep

This comment has been minimized.

Show comment
Hide comment
@flying-sheep

flying-sheep Apr 18, 2018

if i could at least activate all tags and branches as rtd versions, i’d be happy.

right now there’s no way to test a doc change without releasing a beta version on github (i.e. using a tag that looks like a version)

flying-sheep commented Apr 18, 2018

if i could at least activate all tags and branches as rtd versions, i’d be happy.

right now there’s no way to test a doc change without releasing a beta version on github (i.e. using a tag that looks like a version)

@stsewd

This comment has been minimized.

Show comment
Hide comment
@stsewd

stsewd Apr 18, 2018

Member

@flying-sheep

right now there’s no way to test a doc change without releasing a beta version on github (i.e. using a tag that looks like a version)

You can actually build a branch/tag that hasn't a version format. If you create a branch/tag named test, that would be listed under the versions section, then you can activate and built it. If that isn't what you mean, sorry I did not understand it.

Member

stsewd commented Apr 18, 2018

@flying-sheep

right now there’s no way to test a doc change without releasing a beta version on github (i.e. using a tag that looks like a version)

You can actually build a branch/tag that hasn't a version format. If you create a branch/tag named test, that would be listed under the versions section, then you can activate and built it. If that isn't what you mean, sorry I did not understand it.

@flying-sheep

This comment has been minimized.

Show comment
Hide comment
@flying-sheep

flying-sheep Apr 19, 2018

This is what I mean, but I don’t see the tag appear in the versions section.

flying-sheep commented Apr 19, 2018

This is what I mean, but I don’t see the tag appear in the versions section.

@stsewd

This comment has been minimized.

Show comment
Hide comment
@stsewd

stsewd Apr 20, 2018

Member

@flying-sheep rtd doesn't pull automatically tags via webhooks (#3546), so you need to build any current version (like latest for example) in order to rtd pull the new tags/branches.

If that doesn't work, please open a new issue with your rtd project url.

Member

stsewd commented Apr 20, 2018

@flying-sheep rtd doesn't pull automatically tags via webhooks (#3546), so you need to build any current version (like latest for example) in order to rtd pull the new tags/branches.

If that doesn't work, please open a new issue with your rtd project url.

@flying-sheep

This comment has been minimized.

Show comment
Hide comment
@flying-sheep

flying-sheep commented Apr 20, 2018

Thanks!

@ericholscher

This comment has been minimized.

Show comment
Hide comment
@ericholscher

ericholscher Jun 14, 2018

Member

I just had a realization for this, which is that GH lets us access the PR's for a project as a remote:

https://gist.github.com/piscisaureus/3342247

You can then checkout a PR on the repo in a normal fashion:

git reset --hard origin/pr/95

This would let us treat PR's as just another type of Version (Tag/Branch/Pull Request) and use the existing RTD modeling to support them. We could then have a custom syncer for PR's that pushes the content into an S3-type bucket instead of keeping them around forever, and perhaps get this feature done without too much work.

Member

ericholscher commented Jun 14, 2018

I just had a realization for this, which is that GH lets us access the PR's for a project as a remote:

https://gist.github.com/piscisaureus/3342247

You can then checkout a PR on the repo in a normal fashion:

git reset --hard origin/pr/95

This would let us treat PR's as just another type of Version (Tag/Branch/Pull Request) and use the existing RTD modeling to support them. We could then have a custom syncer for PR's that pushes the content into an S3-type bucket instead of keeping them around forever, and perhaps get this feature done without too much work.

@jakirkham

This comment has been minimized.

Show comment
Hide comment
@jakirkham

jakirkham Jun 14, 2018

Contributor

Yeah there is a /merge and /head under that. The first is a generated merge commit with the base branch (presumably master). The second is the latest commit on the PR.

Can also get both using git fetch and optionally attach branches to them locally. We use this a lot in conda-forge. Happy to answer questions.

Contributor

jakirkham commented Jun 14, 2018

Yeah there is a /merge and /head under that. The first is a generated merge commit with the base branch (presumably master). The second is the latest commit on the PR.

Can also get both using git fetch and optionally attach branches to them locally. We use this a lot in conda-forge. Happy to answer questions.

@gaborbernat

This comment has been minimized.

Show comment
Hide comment
@gaborbernat

gaborbernat Jun 19, 2018

Contributor

@ericholscher what you say sounds like a plan. Is this still in blocked given that, or has moved over to needs help/work phase? 👍

Contributor

gaborbernat commented Jun 19, 2018

@ericholscher what you say sounds like a plan. Is this still in blocked given that, or has moved over to needs help/work phase? 👍

@ericholscher

This comment has been minimized.

Show comment
Hide comment
@ericholscher

ericholscher Jun 19, 2018

Member

I believe this should be unblocked.

Member

ericholscher commented Jun 19, 2018

I believe this should be unblocked.

@ericholscher

This comment has been minimized.

Show comment
Hide comment
@ericholscher

ericholscher Jun 19, 2018

Member

There are still some design decisions, but I think at least for GitHub, we have a path forward.

Member

ericholscher commented Jun 19, 2018

There are still some design decisions, but I think at least for GitHub, we have a path forward.

@KengoTODA

This comment has been minimized.

Show comment
Hide comment
@KengoTODA

KengoTODA Sep 15, 2018

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.

KengoTODA commented Sep 15, 2018

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment