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
#11573 Remove Trac documentation build tools #11576
Conversation
37bdecd
to
2e84d94
Compare
I think that this is ready for review needs-review |
39ef680
to
f20808c
Compare
ok... so this is ready for review. please review |
testing please review |
testing again |
please review |
this is ready |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. This looks like a nice collection of baggage to jettison. I left a couple minor comments inline but they really just demonstrate my incomplete understanding of how RTD works. And they're on deleted code so I don't think there's even any comments to add to the implementation.
Looking at the PR build on RTD it seems to be working fine - so please go ahead and merge.
api_base_url = "/{}/{}/api/".format( | ||
os.environ["READTHEDOCS_LANGUAGE"], | ||
os.environ["READTHEDOCS_VERSION"], | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On RTD, we used to set this api_base_url
. Now we're always on RTD and we don't set it? What will api_base_url
be?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
api_base_url - was used as a hack to generate links from Sphinx to pydoctor.
But now pydoctor generated an intersphinx object inventory, an Sphinx can use that for the links.
So the api_base_url
is never used and should never be used.
Its usage was removed a long time ago, but we forgot to clean this part.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah... and we now run the same build process locally and on read the docs
To keep it simple.
In the past, we used to have a custom Sphinx theme, with the goal of integrating with the old website.
But we no longer have an old website ... and the docs are only hosted on Read The docs, with the default RTD theme.
"https://priority.readthedocs.io/en/stable/objects.inv", | ||
"https://zopeinterface.readthedocs.io/en/latest/objects.inv", | ||
"https://automat.readthedocs.io/en/latest/objects.inv", | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we get to drop these because of the upgraded Sphinx dependency?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question.
This is a drive-by fix.
This list was used by pydoctor for API docs.
It was moved to setup.cfg in an older PR ...but I forgot to also delete this.
many thanks for the review. I have enabled auto-merge. I hope it will be green soon. |
Scope and purpose
Fixes #11573
We no longer use twisted-web to host the doccs or Trac theme to integrate the docs to trac theme.
We also no longer build the documentation to be distributed on a separate site.
We only have Read The Docs.
This is cleanup for the code that is no longer used.
The docs are now hosted on Read The docs with build defined in
.readthedocs.yml
and 'docs/conf.py`The docs are generated for dev using tox via
tox -e narrativedocs
that calls sphinx-build directly.I think that by removing the script, it should be easier to understand how the docs are generated.
We no longer have some automated tests, but at the same time, we no longer have custom Sphinx theme or custom extensions that we need to mantain.
How to test
tox -e narrativedocs
Contributor Checklist:
tox -e lint
to format my patch to meet the Twisted Coding Standard#
character).The first line is automatically generated by GitHub based on PR ID and branch name.
The other lines generated by GitHub should be replaced.