Opened 4 years ago

Closed 9 months ago

Last modified 9 months ago

#4500 enhancement closed fixed (fixed)

Use Sphinx for Twisted Documentation

Reported by: khorn Owned by: glyph
Priority: normal Milestone: Lore to Sphinx
Component: core Keywords: documentation
Cc: thijs, gtaylor, hawkowl@… Branch: branches/sphinx-conversion-4500-3
(diff, github, buildbot, log)
Author: khorn, dreid Launchpad Bug:

Description (last modified by thijs)

Twisted should use Sphinx for all long-form documentation (basically everything but API docs).

This ticket is for handling that conversion.

Comments, discussion, and reviews on this ticket should be about overall issues relating to the conversion.

Markup errors in the conversion branch should be handled in separate "markup review" tickets. This is so we can break up the task of manually reviewing all the markup and one person doesn't get stuck doing it all.

Markup review tickets will be listed in the comments of this ticket once there is a branch to review. (after the 10.1 release)

Download

The lore2sphinx tool can be downloaded from https://bitbucket.org/khorn/lore2sphinx.

Change History (57)

comment:1 follow-up: Changed 4 years ago by exarkun

Building the Lore-format documentation is currently part of the continuous integration employed in Twisted's development process.

At some point we need to set up something that does the equivalent of the [http://buildbot.twistedmatrix.com/builders/documentation/builds/512/steps/process-docs/logs/stdio Lore process docs step. I'd like to do this even before we've actually switched over, so we can use it to tell us about any problems in branches related to the conversion.

comment:2 in reply to: ↑ 1 Changed 4 years ago by thijs

  • Cc thijs added

Replying to exarkun:

At some point we need to set up something that does the equivalent of the [http://buildbot.twistedmatrix.com/builders/documentation/builds/512/steps/process-docs/logs/stdio Lore process docs step. I'd like to do this even before we've actually switched over, so we can use it to tell us about any problems in branches related to the conversion.

See #4225 that seems related.

comment:3 Changed 4 years ago by exarkun

One of the steps for lore2sphinx is to run sphinx-quickstart. This prompts for a lot of information, some of which I don't have. I guessed and used defaults for a lot and got far enough so that I could run "make html" which threw out hundreds of warnings and errors, for example:

twisted-sphinx/source/projects/core/index.rst:: WARNING: document isn't included in any toctree
twisted-sphinx/source/projects/words/howto/im.rst:61: (ERROR/3) Unknown interpreted text role "api".
twisted-sphinx/source/projects/web2/examples/index.rst:66: WARNING: download file not readable: projects/web2/examples/auth/credsetup.py

So I think I must have done something wrong. Some more instructions would be helpful.

comment:4 Changed 4 years ago by jml

  • Milestone set to Lore to Sphinx

comment:5 Changed 4 years ago by khorn

@exarkun:

The lore2sphinx repo includes an already-quickstarted sphinx project in the profiles/twisted directory. While this will need to be modified a bit when doing the "real" conversion, the intention was to provide a common baseline for people to test it against during development, though I don't think anyone but thijs ever did.

Also, keep in mind lore2sphinx was intended to be a "run it once for real" tool. Even then it's not perfect, and making it so would likely consume the rest of my useful programming career accounting for special cases, etc. When I run it and then use sphinx-build from either Sphinx 0.6.5 or Sphinx 1.0b2, I get just 13 errors (as of last week). My feeling is that those can be fixed manually.

comment:6 Changed 4 years ago by khorn

#4553 was a duplicate of this ticket

comment:7 Changed 4 years ago by exarkun

Also, keep in mind lore2sphinx was intended to be a "run it once for real" tool.

I know. You've mentioned this too many times for me to ever be able to forget it. :) Despite that, I don't see why we shouldn't be able to run it as many times as we want leading up to the actual conversion. And we need to deal with outstanding branches with changes to the lore source files somehow. It's possible that manually rewriting the docs is the easiest way to go here, but I don't think we should completely throw away the idea of being able to automate at least part of the conversion of such changes.

Even then it's not perfect, and making it so would likely consume the rest of my useful programming career accounting for special cases, etc.

I'm definitely not pushing for this.

When I run it and then use sphinx-build from either Sphinx 0.6.5 or Sphinx 1.0b2, I get just 13 errors (as of last week).

This is more interesting. I used Sphinx 1.0b2. Why did I get hundreds of errors? I suppose it must have had something to do with some of those arguments I gave to sphinx-quickstart. I'll try again with the pre-started project in the lore2sphinx repository.

comment:8 Changed 4 years ago by khorn

I think using the pre-quickstarted project will help. What kinds of errors were you seeing?

The most likely possibility I can think of is that the docs are full of links to example files, which the fabfile and/or the l2sbuilder.py script copy as an intermediate step in the overall conversion process. If you skip this step, you'll see a whole lot of "I can't find this file" type warnings from Sphinx.

comment:9 Changed 4 years ago by exarkun

the fabfile and/or the l2sbuilder.py script copy as an intermediate step

What step is this? I don't see it documented anywhere.

comment:10 Changed 4 years ago by khorn

Well, it's documented in the fabfile and l2s_builder code :)

But I'm afraid that's about it.

comment:11 Changed 4 years ago by khorn

okay, folks, the plan has changed slightly...

exarkun has created a buildbot which runs lore2sphinx on the current lore sources and creates Sphinx output.

quote from exarkun's email:

At last I've got a buildbot set up generating the sphinx docs.  The
build results can be seen here:

http://buildbot.twistedmatrix.com/builds/sphinx-html-15615/

(or with a different revision number for different revisions; the
containing directory is browseable).

In order to minimize the pain of the final conversion, we want to see if we can get the results of the buildbot to look as good as possible before "pulling the trigger".

So we'll be creating "chunk tickets" for that...I'll post the list here once I've created them.

comment:12 Changed 4 years ago by khorn

Here's a list of the chunk tickets:

  • projects/conch - #4566
  • projects/core/development - #4567
  • projects/core/howto (except ‘tutorial/’) - #4568
  • projects/core/howto/tutorial - #4569
  • projects/historic - #4570
  • projects/lore - #4571
  • projects/mail - #4572
  • projects/names - #4573
  • projects/pair - #4574
  • projects/vfs - #4575
  • projects/web (everything except web-in-60) - #4576
  • projects/web/web-in-60 - #4577
  • projects/web2 - #4578
  • projects/words - #4579

comment:13 Changed 4 years ago by khorn

NOTE: it needs to be possible to build the docs without the docs for vfs and web2, since they are not currently (and should not be) published to the website

It should be possible to handle this using one of the "conditional content" mechanisms in Sphinx.

comment:14 Changed 4 years ago by gtaylor

  • Cc gtaylor added

Sorry if this isn't the right place to ask, but where can we go to learn more about helping with this?

comment:15 Changed 4 years ago by khorn

@gtaylor:

Basically, the plan has remained the same. I think everyone just got busy (I certainly did). We finish reviewing and committing the "chunk" tickets listed in the comments above to make the Lore source convert as close to correctly as possible. Then we convert it to Sphinx using lore2sphiunx and get it into SVN. Then we set up a new set of "chunk" tickets for reviewing and fixing the RestructuredText source.

One possible obstacle is that the lore2sphinx buildbot seems to have broken sometime in the last couple of months. We need to see if we can get that fixed.

As for helping, you can try reviewing and fixing the tickets above, or perhaps post to the Twisted mailing list, which is probably an easier forum for discussions like this.

comment:16 Changed 4 years ago by thijs

  • Description modified (diff)

Another ticket: #4621 - lore2sphinx should make pretty links to included files.

comment:17 follow-ups: Changed 4 years ago by khorn

OK, current status (following the Pycon 2011 sprints).

  • all the markup tickets for modifying the lore sources to make the generated Sphinx sources look nicer are done
  • the "apilinks" Sphinx extension has been moved into PyDoctor (as "apilinks_sphinxext.py"). We need to make sure that it will get installed with PyDoctor in a way that Sphinx will be able to find it (it needs to be on the Python path)
  • the "traclinks" extension is available in sphinx-contrib
  • there will likely be another extension which will be used to make the sphinx literalinclude directives look a bit nicer (have nice links to the included files)

remaining work:

  • the twisted release automation needs to be updated to use Sphinx rather than Lore. This could be tricky...

comment:18 Changed 4 years ago by khorn

  • Author set to khorn
  • Branch set to branches/sphinx-conversion-4500

(In [31151]) Branching to 'sphinx-conversion-4500'

comment:19 Changed 4 years ago by glyph

exarkun pointed out that there should be no particular pressure to do this in time for the 11.0 release. Whenever it's completely done, even if that's just a day after the release is finished, we can re-generate the documentation from 11.0 and put it on the website immediately. There will be no need to do another release just to make that happen.

In fact, a separate announcement might be even better publicity-wise, so that we can emphasize the newly indexed documentation on its own.

comment:20 in reply to: ↑ 17 Changed 3 years ago by thijs

Replying to khorn:

remaining work:

  • the twisted release automation needs to be updated to use Sphinx rather than Lore. This could be tricky...

So looking at the Lore2Sphinx timeline, shows there's 2 tickets left, this one and #4621. Is there a ticket for twisted release automation update? Anything else? Hopefully this can go into 11.1 :)

comment:21 in reply to: ↑ 17 Changed 3 years ago by thijs

Replying to khorn:

  • there will likely be another extension which will be used to make the sphinx literalinclude directives look a bit nicer (have nice links to the included files)

Which is reported in #4621.

comment:22 Changed 3 years ago by khorn

  • Keywords review added

Please review.

Note that so far this is only some changes to the release mgmt stuff. I'd like to get that reviewed before switching over the actual documentation.

comment:23 Changed 3 years ago by khorn

  • Owner khorn deleted

comment:24 Changed 3 years ago by glyph

  • Owner set to glyph
  • Status changed from new to assigned

Reviewing.

comment:25 Changed 3 years ago by glyph

  • Keywords review removed
  • Owner changed from glyph to khorn
  • Status changed from assigned to new

Hi khorn! Thanks for writing this branch.

  1. First and foremost, I think that we should move this branch out to a different ticket. We can merge in some release-automation code to build the sphinx documentation before we actually commit all of the sphinx input, since reviewing the ReST sources may come up with other, unrelated issues. The more stuff we can get merged in small chunks, the better. Alternately we could open a different ticket for the ReST source migration, but this ticket seems to be pretty clearly aimed at that purpose. So you may want to respond to this review on a different ticket.
  2. There's tons of trailing whitespace in this branch; it should be cleaned up.
  3. SphinxBuilder.__doc__ isn't very clear.
    1. %SPHINXBUILD%? Give me a break. Hopefully nobody will ever actually run this on Windows (the release process definitely isn't supported there) so this syntax is pretty meaningless.
    2. The docstring should really say what the expectations for ALLSPHINXOPTS and BUILDDIR are.
    3. SphinxBuilder.build.__doc__ doesn't explain its parameters terribly well. What is the expected structure of docDir and buildDir? Where's the sphinx project supposed to be?
  4. runCommand takes a list of strings, not a string. It happens to work as-is only because Popen (incorrectly) allows you to pass in a string. There are all kinds of potential problems with quoting this input.
  5. Don't put # TODO markers in the code. If something needs to be done, file a new ticket. If not, just add an implementation note.
  6. The comments about text/latex/pdf builders should similarly also be separate tickets.
  7. Docstrings should always be """-style strings, not '''. Many of the tests use the latter style.
  8. @ivar declarations ought to go in the class docstring, not method docstrings, so please move the ones from setUp. Thanks for thinking to document those, though :-).
  9. For loops in tests are a bad idea. There's always the chance that you'll accidentally loop over nothing or somehow abort the loop early, and get a false positive. Even when the tests do their job correctly and fail on some bad data, a for loop means that when you get a failure you have to do forsensics to figure out which data element it failed on, since the traceback doesn't tell you. So, instead of writing tests like this:
    data = [[a, b], [c, d], [e, f]]
    for thunk in data:
        assertSomething(thunk)
        assertSomethingElse(thunk)
    
    write them like this:
    def doAsserts(thunk):
        assertSomething(thunk)
        assertSomethingElse(thunk)
    doAsserts([a, b])
    doAsserts([c, d])
    doAsserts([e, f])
    
  10. SphinxBuilderTests docstring looks pretty incoherent: was this a debug / note to self that never got updated?
  11. Some of the names are not coding-standard compliant. They should be confContent, indexContent, and test_failToBuild respectively.
  12. A matter of personal taste: triple-quoted strings (aside from docstrings) should be indented so that their contents are further-indented than their enclosing suite. Since you're using textwrap to re-align them anyway, I figure that should be easy.
  13. I'd really like it if this would include a command-line tool to update documentation in the right shape for the web site so that the release process doesn't develop another hole, which is currently plugged by an untested crappy thing on a wiki page. At the very least some new "documentation" (ha ha) of this form needs to be written.
  14. As a minor detail, the tests fail just about everywhere ;-) I believe this is because of the aforementioned string/list of strings Popen thing.

Thanks again Kevin for continuing your work on this. I am looking forward to putting this ticket behind us :-).

comment:26 Changed 3 years ago by khorn

splitting out the release automation stuff to #5312

comment:27 Changed 14 months ago by hawkowl

  • Cc hawkowl@… added

comment:28 Changed 14 months ago by glyph

#5312 is now closed. Anyone happen to know what the next action here is?

comment:29 Changed 9 months ago by glyph

  • Branch changed from branches/sphinx-conversion-4500 to branches/sphinx-conversion-4500-2

comment:30 Changed 9 months ago by glyph

  • Keywords review added
  • Owner khorn deleted

Ready for review.

You can already see it set up on http://twisted.readthedocs.org/en/latest/

comment:31 Changed 9 months ago by Alex

  • Keywords review removed
  • Owner set to glyph

[41202] added some new documentation which needs to be ported to swift before this can land.

comment:32 follow-up: Changed 9 months ago by exarkun

I am (genuinely) not sure if taking this out of review for r41202 will ultimately help land this ticket. I hope someone will address the general point of what this change means for outstanding work that includes changes to the documentation that is being converted. We have a lot of outstanding branches and patches and a few of them are bound to have documentation changes.

If the answer is that people just need to manually port those changes over to reStructuredText, it'd be good to at least have that written down somewhere. I tried to look at the branch to see if this has already been addressed but given a 62k+ line diff it's a bit hard to find things. :)

comment:33 Changed 9 months ago by dreid

  • Author changed from khorn to khorn, dreid
  • Branch changed from branches/sphinx-conversion-4500-2 to branches/sphinx-conversion-4500-3

(In [41230]) Branching to sphinx-conversion-4500-3

comment:34 in reply to: ↑ 32 Changed 9 months ago by glyph

Replying to exarkun:

I am (genuinely) not sure if taking this out of review for r41202 will ultimately help land this ticket. I hope someone will address the general point of what this change means for outstanding work that includes changes to the documentation that is being converted. We have a lot of outstanding branches and patches and a few of them are bound to have documentation changes.

I think taking this out of review probably wasn't the right call, because the review of this branch is a little bit special and weird and it's a bit more time-sensitive than most changes. However, Alex was right in that it shouldn't have landed.

If the answer is that people just need to manually port those changes over to reStructuredText, it'd be good to at least have that written down somewhere. I tried to look at the branch to see if this has already been addressed but given a 62k+ line diff it's a bit hard to find things. :)

It's a race. If the other branches land before this one, we should update this one to include the new documentation, just so we can get buildbot to do its job.

The solution is indeed manual porting documentation over to sphinx ReST. In most cases, the amount of documentation one branch is adding isn't too onerous. Additionally, the lore2sphinx buildbot will keep running for a while, and developers can force a build there for their branch and grab the ReST source for their converted document as a starting point.

And finally, since I want to make very very sure that we do not punish anyone for having written documentation with extra work, let me make my personal voluteering explicit here; if anyone has a patch or branch with documentation changes that need to be ported over, I will personally volunteer to shepherd that change through the process, from rewriting the documentation to doing the merge forward to making sure all the ReST has appropriate markup and compiles without warnings.

I can put this on the mailing list if you think that would be helpful.

comment:35 Changed 9 months ago by glyph

  • Keywords review added

Back to review. The pyflakes builder seems to be producing totally spurious output, and it has been for the last couple of reviews that I've looked at.

Otherwise looks like it's in good shape.

comment:36 Changed 9 months ago by glyph

  • Owner glyph deleted

comment:37 Changed 9 months ago by glyph

Dear reviewers,

This branch looks quite big and daunting, but is actually quite easy to review. Don't bother reviewing all 60,000 lines of change here: almost all of it is mechanically produced by the output of lore2sphinx, and has been at least somewhat reviewed in the past by people looking at documents. Some of the documents may even have transcription errors in them; that's OK, and we can fix those in subsequent tickets.

The things to look for are:

  1. Make sure the release automation toolchain still works as expected. ./bin/admin/build-docs should still work and produce some HTML documentation. (Needless, I hope, to say, you will still need sphinx installed.)
    1. Note that this branch does make a change to release automation, in that subproject tarballs are no longer distributed with their documentation; however, the documentation is written with hyperlinks between projects and was never really tested or reviewed in that configuration anyway. Review the artifacts that are produced. (dreid is actually working on an issue with PDF generation right now, so hopefully that will be resolved before a reviewer looks at all this. I'll leave it to the reviewer's discretion, but if it's not, given the rather light usage of the PDF it might be better to land this without a PDF and fix that later than to hold up on all the improvements Sphinx and Read The Docs will give us, especially since Sphinx and RTD will produce a far superior long-form reading format for our modern era, epub.)
    2. Have a look at the generated documentation on Read The Docs.. You don't need to review this too carefully, mostly just look at it and be excited about how cool it looks to motivate getting this through review quickly :-).
  2. Spot-check the output to make sure there are no egregious errors. Don't review the all of new documentation for quality, we can address issues like index layout in future changes; however, if, for example, the twisted.names documentation is entirely missing we should probably fix that before this lands.
  3. Review the few code changes that there are (in the release automation toolchain) for the usual docstring/test coverage/coding standard issues.

comment:38 Changed 9 months ago by dstufft

  • Owner set to dstufft
  • Status changed from new to assigned

comment:39 follow-up: Changed 9 months ago by dstufft

  1. I believe that the coding standard states that docstrings should use """ and not ''' however the docstrings for docs/_extensions/apilinks.py and docs/_extensions/traclinks.py are using the latter.
  2. There are a number of files that do not end with a new line.
  3. The sphinx configuration uses twisted.version.short() for the "version" configuration. The documentation for Sphinx suggests that this should be X.Y while the release should be X.Y.Z with alpha/beta/tags. Looking at the docstrings for twisted.version.short() it appears that this is going to include prerelease information. So the version configuration should at least use twisted.version.base() if not something new like twisted.version.series.
  4. I'm guessing there isn't supposed to be an l in the traclinks_base_url value of "https:l//twistedmatrix.com/trac"
  5. Earlier above glyph stated that he doesn't expect this to run on Windows so perhaps it would make sense to remove the windows bat file? Or was that comment perhaps directed only at SphinxBuilder and the release automation?
  6. This might be desired, but I just thought I'd mention that I find the docs/doc dirs confusing to read. It's hard to keep straight which one is the source directory and which one is the build directory.
  7. The other ./bin/admin/build-* commands give a useful error message when called without arguments, however build-docs simply gives an IndexError.

comment:40 follow-up: Changed 9 months ago by dstufft

One thing that I thought I'd mention too, I don't think it's required, but I find that sphinx config files are way easier to read if I delete all the commented out stuff that they include by default. Might be a nice enhancement.

comment:41 in reply to: ↑ 40 Changed 9 months ago by glyph

Replying to dstufft:

One thing that I thought I'd mention too, I don't think it's required, but I find that sphinx config files are way easier to read if I delete all the commented out stuff that they include by default. Might be a nice enhancement.

That stuff was left in on purpose so that the configuration file would be as close as possible to a 'sphinx quickstart'. Perhaps worth discussing on a future ticket :).

-glyph

comment:42 Changed 9 months ago by dstufft

  • Keywords review removed
  • Owner changed from dstufft to dreid
  • Status changed from assigned to new

comment:43 Changed 9 months ago by tom.prince

This might be desired, but I just thought I'd mention that I find the docs/doc dirs confusing to read. It's hard to keep straight which one is the source directory and which one is the build directory.

If this is just done to keep buildbot happy, it would be fairly easy to make buildbot smarter to deal with different directories.

comment:44 Changed 9 months ago by dreid

  • Owner changed from dreid to glyph

comment:45 follow-up: Changed 9 months ago by rwall

I noticed a few things while experimenting in log:branches/custom-server-documentation-6864/docs/projects/names/howto/names.rst

(probably not things that should block this ticket...but here they are anyway)

The external links don't seem to be rendered properly in this document:

The code examples no longer have line numbers by default eg

Will we have to explicitly ask for line numbers using the linenos attribute?

I tried doing that, but also encountered the following bug which causes line numbers to always start at 1, even if we asked to display only certain lines

It's a shame that included listings now require two lines (include and download link) instead of one

<a href="listings/names/example-domain.com" class="py-listing">Zone file</a>

vs

:download:`example-domain.com <listings/names/example-domain.com>`

.. literalinclude:: listings/names/example-domain.com

And notice that the original link text "Zone file" has been changed to "example-domain.com" which isn't as useful.

And it's displayed simply as "example-domain.com" where as in the current docs it also includes the relative link -- which looks nicer and is useful for finding that file in the twisted source directory.

The url fragment IDs (is that what they're called?) have also changed name eg

The new names are much nicer, but I've often put links containing fragment IDs in ticket comments and in source code comments.
Will it be possible to redirect these once the new docs go live?
I guess they are only available to the client, so it would require some JavaScript.

comment:46 in reply to: ↑ 45 ; follow-up: Changed 9 months ago by glyph

Replying to rwall:

I noticed a few things while experimenting in log:branches/custom-server-documentation-6864/docs/projects/names/howto/names.rst

(probably not things that should block this ticket...but here they are anyway)

Thanks for listing these and also thanks for explicitly mentioning that they should not block :-).

The external links don't seem to be rendered properly in this document:

Weird. Did you notice that in other documents as well?

The code examples no longer have line numbers by default eg

Will we have to explicitly ask for line numbers using the linenos attribute?

Do we have any documentation that makes reference to line numbers? My looking around did not yield any, and I vaguely recall a discussion when we were originally doing this, where we decided against line numbers (since I'm pretty sure the original "chunk review" process would have caught it if it were pervasive at all).

I tried doing that, but also encountered the following bug which causes line numbers to always start at 1, even if we asked to display only certain lines

It's a shame that included listings now require two lines (include and download link) instead of one

Yup, ReST is a crappy input format, docutils is a crappy toolchain, etc etc, but progress must march on regardless :-).

And notice that the original link text "Zone file" has been changed to "example-domain.com" which isn't as useful.

An instance of the same 'external links' file.

And it's displayed simply as "example-domain.com" where as in the current docs it also includes the relative link -- which looks nicer and is useful for finding that file in the twisted source directory.

Right. I'm a little surprised that lore2sphinx isn't preserving this information automatically.

The url fragment IDs (is that what they're called?) have also changed name eg

The new names are much nicer, but I've often put links containing fragment IDs in ticket comments and in source code comments.
Will it be possible to redirect these once the new docs go live?

Hypothetically it's possible, but I think we are just giving up on this on purpose. First and foremost, it would be a huge amount of work and there just isn't a lot of benefit. The #autoN links were never stable anyway between documentation versions; the only reason you can sort of rely on them is that they didn't change a lot, so even if we went to the effort of writing the JS to do this, we'd just get a correct #autoN enumeration for documents which didn't change their number of sections between the last release and this one.

More concerning to me (in terms of links continuing to work) is the fact that we added this extra projects/ directory to the hierarchy, which means that the "go to the current version of this document" links in older versions of the docs (for example the one on the top of this page) will now 404. That seems worth fixing.

comment:47 Changed 9 months ago by dreid

I filed #6900 about addressing the issue of the current XHTML Documentation Standard which will be incorrect when this branch lands.

comment:48 in reply to: ↑ 46 Changed 9 months ago by rwall

Replying to glyph:

The external links don't seem to be rendered properly in this document:

Weird. Did you notice that in other documents as well?

I didn't actually look. That just happens to be a doc that I wrote.

I also noticed that the listings haven't been included on that page.

Maybe there's something weird about my original xhtml markup.

comment:49 follow-up: Changed 9 months ago by rwall

Oh and I also see there's another error on that page (a <del> tag which can probably be discarded)

Sorry.

comment:50 in reply to: ↑ 49 Changed 9 months ago by glyph

Replying to rwall:

Oh and I also see there's another error on that page (a <del> tag which can probably be discarded)

Sorry.

Some small amount of junk like that is acceptable and we can fix it in subsequent tickets.

comment:51 Changed 9 months ago by glyph

So it looks like we're down to a couple of substantive issues:

  1. Links to the most recent version of the docs from older versions will break upon the next release. We should eliminate the projects subdirectory and that will basically be fixed, as far as I can tell.
  2. lore2sphinx appears to have discarded some information, i.e. the names of external link targets. We did explicitly decide that it was OK to drop the names of listings in a previous discussion. While this is unfortunate, given that internal links (within the documentation itself) and API links already work (see here for example, where "listenTCP" and "endpoints" are regular links and not full URLs), I think it would be less work at this point to press ahead and do a couple of ReST updates to fix the documents that have lots of external link than to re-do all the work that dreid did in the branch to fix the toctrees and all the sphinx warnings.

comment:52 in reply to: ↑ 39 Changed 9 months ago by glyph

  • Keywords review added

Replying to dstufft:

  1. I believe that the coding standard states that docstrings should use """ and not ''' however the docstrings for docs/_extensions/apilinks.py and docs/_extensions/traclinks.py are using the latter.

Yeah, Kevin had some weird ideas about how to format code, all of lore2sphinx is neither quite Twisted style nor quite PEP8 style.

  1. There are a number of files that do not end with a new line.

Fixed.

  1. The sphinx configuration uses twisted.version.short() for the "version" configuration. The documentation for Sphinx suggests that this should be X.Y while the release should be X.Y.Z with alpha/beta/tags. Looking at the docstrings for twisted.version.short() it appears that this is going to include prerelease information. So the version configuration should at least use twisted.version.base() if not something new like twisted.version.series.

I wish that sphinx would explain what this is for; I can't imagine ever not wanting to know exactly what version I'm generating documentation for. But fixed nevertheless.

  1. I'm guessing there isn't supposed to be an l in the traclinks_base_url value of "https:l//twistedmatrix.com/trac"

Yup. Fixed.

  1. Earlier above glyph stated that he doesn't expect this to run on Windows so perhaps it would make sense to remove the windows bat file? Or was that comment perhaps directed only at SphinxBuilder and the release automation?

That, and the Makefile, are artifacts of sphinx-quickstart that are provided as a convenience for crazy people who think that things like Makefiles and batch files are "a convenience" (like whoever wrote sphinx-quickstart). They're not part of the release process. If someone working on Windows wants to contribute docs and is used to sphinx's idiom of writing out a batch file, we'd like them to be comfortable, so that's there on purpose .

  1. This might be desired, but I just thought I'd mention that I find the docs/doc dirs confusing to read. It's hard to keep straight which one is the source directory and which one is the build directory.

So, this is there to keep the buildbot happy, as Tom guessed. This is something that could definitely be fixed later, since your VCS should make it pretty clear that it's the output locally, and if you just use the makefile or batch file you won't be building into that directory anyway.

  1. The other ./bin/admin/build-* commands give a useful error message when called without arguments, however build-docs simply gives an IndexError.

Thanks for noticing, but I think we can pass on this too for now. Also separately I'd like to fix up that tool so that it actually doesn't even need any command line arguments at all and just works on the computed relative source directory.

So that's all the review feedback. dstufft was not terribly clear about whether any of his feedback was a blocker, and rwall seems happy with deferring his feedback; are we ready to call this done?

comment:53 Changed 9 months ago by dstufft

Oops sorry I forgot to follow up on this, sorry! The only things I considered blockers you've fixed. So this is good to go from my end.

comment:54 Changed 9 months ago by dstufft

  • Owner changed from glyph to dstufft
  • Status changed from new to assigned

comment:55 Changed 9 months ago by dstufft

  • Keywords review removed
  • Owner changed from dstufft to glyph
  • Status changed from assigned to new

comment:56 Changed 9 months ago by glyph

  • Resolution set to fixed
  • Status changed from new to closed

(In [41261]) Merge sphinx-conversion-4500-3: Convert Lore to Sphinx documentation.

Author: glyph, khorn, dreid, Alex
Reviewer: dstufft
Fixes: #4500

This change is the big conversion that moves all documentation in the
repository to Sphinx and removes all Lore documentation sources.

This is not perfect, but should be a big improvement. Further fixes
should happen in subsequent branches, on the ReST sources.

comment:57 Changed 9 months ago by hawkowl

Are we supposed to still be putting markup review tickets in these comments?

In this case, I have started with #6943, using a small script to find out what won't end up with merge conflicts.

Note: See TracTickets for help on using tickets.