Do not build documentation in Circle CI, Read the Docs handles that 💯

[benjaoming]

• Removed tox entry in Circle CI configuration
• Warnings will now propagate to failed builds on Read the Docs

I think we should also remove the tox configuration, no reason to maintain it unless someone uses it actively in their dev env? Our develop documentation doesn't mention it: https://dev.readthedocs.io/en/latest/docs.html

A side-note to this change: I think it's confusing that we had this build running. I kept wondering why - is there something that Read the Docs CI doesn't do?

[ericholscher]

I think this makes sense. Probably worth a note in the tox.ini saying that these 2 aren't run in CI, but are still there to be used locally?

[benjaoming]

Added the comment 👍

Wondering still if we need these tox environments, but I kind of like them because we have these custom environments for building the docs (PROJECT=user etc) and this is a nice manifestation for running them.

[ericholscher]

Seems the PR lost the actual changes to tox?

[benjaoming]

You're right, I suspect a bad merge resolution in .circleci/config.yml - I redid the bad merge commit.

[humitos]

If we want to remove these checks from CircleCI, we have to make the Read the Docs CI checks Required by GitHub. Otherwise, we will be able to merge PR when docs are failing.

[benjaoming]

If we want to remove these checks from CircleCI, we have to make the Read the Docs CI checks Required by GitHub. Otherwise, we will be able to merge PR when docs are failing.

That's a really good idea! It can also help understand if this behavior is relevant to conditional build skipping, see: #9791

[ericholscher]

It also looks like our RTD builds are not reporting properly on exit code failures (as seen in this PR), so we'd need to fix that before marking them required too 🙃

[benjaoming]

@ericholscher

It also looks like our RTD builds are not reporting properly on exit code failures (as seen in this PR), so we'd need to fix that before marking them required too 🙃

Could we defer this to #9791 ?

We are having yellow builds in this and other PR regardless of the Circle CI removal. However our docs builds do trigger when there are documentation changes. Since this PR doesn't have any documentation changes, the builds were skipped and never sent back a status (hence "yellow").

Meaning we can move forwards with this change but keeping in mind that we have some WIP on the RTD build skipping feature (if we don't manage to solve it quickly, we should disable build skipping and make the RTD builds required for PR merge)

[ericholscher]

Right, I just think that having a known failing build be required makes PR's harder to use, and non-admins can't merge things I don't think. I'm fine if we want to remove the circleci docs stuff, and keep RTD unrequired, with a note to update that in #9791.

[ericholscher]

I'm good with this, with a note that we should mark this required once we fix the reporting issue 👍

[benjaoming]

Added as a note in #9791 👍