search

FactoryBoy / factory_boy

A test fixtures replacement for Python
https://factoryboy.readthedocs.io/
MIT License
3.48k stars 392 forks source link

Make building the RTD docs a CI check for every PR, and make it required #1076

Open jeffwidman opened 4 months ago

jeffwidman commented 4 months ago

I noticed that there's no RTD CI check that runs on every PR.

I realize there is a check to ensure docs build successfully, but that only checks local builds. Whereas I've found the RTD check useful on other project I help maintain to catch any drift between RTD required configs and local sphinx.

I went to enable this, but I'm not currently an admin on FactoryBoy's RTD instance.

@rbarrois do you mind adding me as an admin on Factory Boy's RTD?

Mind want to ensure @francoisfreitag is also an admin there to minimize bus factor.

Or if there's some reason this check isn't enabled/required, just let me know...

rbarrois commented 4 months ago

Hi Jeff!

Adding the docs checks might be a good idea, although I thought it was already covered in the docs tox target?

jeffwidman commented 4 months ago

Adding the RTD check verifies not only that the docs build locally, but also that RTD can build them.

For example:

  1. currently this project uses setup.py + setup.cfg, but pyproject.toml appears to be the way of the future (https://github.com/pypa/setuptools/issues/3214)... if/when that migration happens, it could possibly require updates on .readthedocs.yaml
  2. there's pinned versions of python/Ubuntu in this repo's .readthedocs.yaml... easy to forget to update them later.

I don't see any downsides to enabling the automated check? Enabling it is a checkbox on the RTD admin page for the project, it doesn't require adding any workflow files in this repo, so it's really easy...

jeffwidman commented 2 weeks ago

nudge @rbarrois on ☝