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
Remove use of API function pages #1496
Conversation
Private test packages are excluded from the API docs build, so there is no documentation page to link to.
These docs were moved to the external Constantly library, along with the code. A link to Constantly's docs remains.
These links were broken. The solution is to use intersphinx instead.
The attempt to use apilinks led to a broken link.
The links to IReactorSocket.adoptDatagramPort in the UDP howto had a space inside the link target that shouldn't be there. A link in the compatibility policy was missing the closing '>' around the label. Two links in the names client tour were missing a space between the link target and label. While Sphinx will parse it correctly without the space, it is an unnecessary inconsistency that makes it harder to convert the links automatically.
This is an easy mistake to make, since the apilinks extension uses the opposite order of the Sphinx link syntax.
Intersphinx does not have a notion of inheritance, so the link must point to the actual definition. In some cases the actual definition has a private name, so maybe pydoctor should create a public alias for those. But for now, a working link to a private name is better than a broken link.
These linked to an existing class, but with a bogus attribute attached.
One contains a typo, the other used the wrong interface name.
Some parts of Twisted's API are published under more than one name. However, pydoctor currently cannot handle that and will pick one of the names.
This will allow us to get accurate deep links, so we don't need function pages anymore. It will also make links to non-existing names errors, instead of having broken links in the output.
This extension is no longer necessary now that we use intersphinx exclusively.
Function pages were a workaround to link to functions from the narrative docs using the apilinks extension. It created a dedicated HTML page for every function/method. Since Twisted has lots of functions and methods, the function pages accounted for nearly 90% of files written by pydoctor, slowing down both the generation and the publishing of the API documentation.
@@ -0,0 +1 @@ | |||
Fix a few dozen broken links to API documentation pages. |
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.
Maybe. "The documentation was updated to use the Sphinx intersphinx extension for linking between our narrative documentation and the Twisted own API documentation."
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.
Your suggestion is a much better description of the PR, but the news fragments are for the release notes and I figured that the only thing the user is going to notice is that there are fewer broken links.
@@ -240,7 +239,7 @@ | |||
intersphinx_mapping = { | |||
"py3": ("https://docs.python.org/3", None), | |||
"zopeinterface": ("https://zopeinterface.readthedocs.io/en/latest", None), | |||
"api": (apilinks_base_url, "../apidocs/objects.inv"), | |||
"api": (api_base_url, "../apidocs/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.
Maybe rename "api" with "twisted" ... to know that this is the twisted on API...
I am not sure... "api" looks very generic... but at the same time, this is just a random name.
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.
The names where can be used in references for disambiguation, but since the API objects have unique names anyway, this isn't used.
Since this inventory only contains API objects and not narrative objects, "api" is specific about which part of Twisted it covers, although the fact that it covers Twisted in the first place is implicit.
Todo for myself - see how link to trial's private API work. |
fabulous job... I will do another review later...but this looks solid work :) |
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.
Excellent work! This can be merged :) Thanks a lot!
Contributor Checklist:
tox -e lint
to format my patch to meet the Twisted Coding Standardreview
to the keywords field in Trac, and putting a link to this PR in the comment; it shows up in https://twisted.reviews/ now.