iGEM-Engineering / iGEM-distribution

Repository for collective design of an iGEM DNA distribution
https://igem-distribution.readthedocs.io
Other
42 stars 20 forks source link

Status of read-the-docs builds isn't included in pull requests #250

Closed noahsprent closed 2 years ago

noahsprent commented 2 years ago

Ideally read-the-docs should be able to detect pull requests to develop and build them so that we can test that the docs will build correctly before we merge the branch. This should be possible, but we need to get it working.

noahsprent commented 2 years ago

Ok so it seems like pull requests are being built (One will only see all the versions if they are an admin of RTD). But they are inactive, which means that the webpage is not served. After activating a branch, even when it is not hidden, it seems that when navigating to the default site the pull request versions don't come up: image But when manually going to the link for the new site e.g. (https://igem-distribution.readthedocs.io/en/add-intro-doc/) the site is there and there is a link to the 'latest' version: image I'm guessing this is because the 'latest' version needs to be rebuilt to see the new link.

I think the default behaviour to not activate or display every branch is fine, as long as we have a webhook that runs doc build testing when the pull request is opened in GitHub. If people haven't built them locally and want to see what they look like, we can activate and hide that specific branch for them on rtd so that they can see it. So we need to figure out how to add that webhook. It may have been one that I deleted... (https://docs.readthedocs.io/en/stable/integrations.html).

noahsprent commented 2 years ago

Ok I don't really know why this isn't working, maybe because I initially set up the GitHub webhook? @vinoo-igem would you be able to try adding the integration? (https://readthedocs.org/dashboard/igem-distribution/integrations/create/) --> GitHub incoming webhook --> Add integration

vinoo-igem commented 2 years ago

I'll have a look at that today, and try to get it sorted

vinoo-igem commented 2 years ago

So I fussed around a bit and changed GitHub's webhook for read-the-docs. It should trigger the webhook for: Branch or tag creation, Branch or tag deletion, and Pull requests. And happy to remove branch related triggers, but just followed the documentation on that.

On read-the-docs side though, I noticed when resyncing webhook for GitHub it gets a failed message.

noahsprent commented 2 years ago

Thanks @vinoo-igem. I've just made a quick commit to #248 to see if it would work now, and although it's building fine on RTD there's still no status update in GitHub. When I go to RTD I get the error:

Could not send GitHub build status report for iGEM-distribution. Make sure you have the correct GitHub repository permissions and your GitHub account is connected to ReadtheDocs.

Is your GitHub account connected to RTD? The only other thing I can think to do if it is is to remove myself and @jakebeal as maintainers.

vinoo-igem commented 2 years ago

When signing up I did not do that as it didn't seem necessary for the integration, just that it would make it easier. I can try again though.

noahsprent commented 2 years ago

It feels like it might be something that can be fixed by knowing a lot about webhooks/integrations but might be automatically sorted out by the account being connected. Sorry this isn't proving to be that smooth, would you be able to give it a try?

vinoo-igem commented 2 years ago

So below are the permissions that read-the-docs would need. Seems fairly boilerplate, but wanted to get any input on concerns before I go ahead to connect accounts. @jakebeal @noahsprent

noahsprent commented 2 years ago

That's fixed it, thanks very much @vinoo-igem! image