Changes between Initial Version and Version 1 of DocumentationAnalysis/Clients/AndrewBennetts


Ignore:
Timestamp:
03/06/2006 07:19:01 AM (9 years ago)
Author:
spiv
Comment:

--

Legend:

Unmodified
Added
Removed
Modified
  • DocumentationAnalysis/Clients/AndrewBennetts

    v1 v1  
     1= Review Details = 
     2 
     3 * Link to document: http://twistedmatrix.com/projects/core/documentation/howto/clients.html 
     4 * Reviewer's name: Andrew Bennetts 
     5 * Review date: 7th March 2006 
     6 
     7= Document Expectations = 
     8 
     9== Intended User == 
     10 
     11Before someone begins using this document, I'd really really like it if they 
     12knew: 
     13 
     14 * more or less what a TCP socket is, and that they connect to endpoints 
     15   specified by host + port. 
     16 * more or less what Deferreds are. 
     17  
     18Once someone has read this document they should understand: 
     19 
     20 * how to write a Twisted program that connects (and uses) a single TCP client. 
     21 * how to write a Twisted program that connects & uses multiple concurrent TCP 
     22   clients. 
     23 * how to write a Twisted program that uses TCP clients concurrently with other 
     24   functionality, like running a TCP server. 
     25 * how to use `ClientCreator` to create try-once client connection. 
     26 * how to use `ReconnectingClientFactory` to make a client that tries to 
     27   reconnect lost connections automatically. 
     28 * how to define custom reconnection logic (in the same manner that 
     29   `ReconnectingClientFactory` does), including how to abort a connection attempt. 
     30 * that Protocols work the same way for servers and clients (even though most 
     31   protocols aren't symmetrical). 
     32 
     33= Document Review = 
     34 
     35== Coverage of subject matter == 
     36 
     37This document seems to cover the following subject matter at least acceptably well: 
     38 
     39 * the relationship of protocol instances to connections (although theoretically 
     40   this doesn't have to be true, in practice most `ClientFactories` do this, so 
     41   the simplification here is fine). 
     42 * it recommends the use of `ReconnectingClientFactory` for reconnection logic, 
     43   and gives an example of using it. 
     44 * the `ircLogBot` example is a good, real example, that does something useful and 
     45   interesting, and demonstrates use of `ClientFactory` and `reactor.connectTCP`. 
     46   (and judging from the number of times I've seen "twistedbot" join #twisted, 
     47   people actually run and play with this code a fair bit). 
     48 
     49This document seems to be attempting to cover the following subject matter, but its coverage is flawed: 
     50 
     51 * the use of the term "configuration" for application state usually held in a 
     52   factory is confusing and probably too abstract. 
     53 * the example in "Simple, single-use clients" is good, except that it's not 
     54   quite a complete, executable example.  It should have a reactor.run() line. 
     55 * "Simple, single-use clients" should give a comprehensive list of methods that 
     56   are specific to `ClientFactories` (or perhaps this could come later in the 
     57   document, but it definitely should be in this document somewhere). 
     58 * "Simple, single-use clients" gives an example that needs to take connector 
     59   arguments, and although it never uses them, it's a bit weird to just never 
     60   mention them or what they are here.  Also, it should say that "reason" is a 
     61   `Failure`; basically, `ClientFactory` should be introduced with a comprehensive 
     62   API overview. 
     63 * The "To connect this `EchoClientFactory` to a server, you could use this code:" 
     64   snippet may as well be part of the main example in this section. 
     65 * The "Reconnection" section starts talking about calling various methods on 
     66   the "`connector`" variable, but never says what it is or links to the API docs 
     67   for `IConnector`. 
     68 * The explanation of `ClientCreator` is too terse -- e.g. it doesn't make it clear 
     69   that it returns a `Deferred` of the connected protocol. 
     70 * The `ircLogBot` example imports `twisted.protocols.irc`, which has been 
     71   deprecated.  It should import `twisted.words.protocols.irc` instead. 
     72 
     73This document ought to be covering the following subject matter but is not: 
     74 
     75 * Never are connectors or the `IConnector` interface explained. 
     76 * The APIs of `ClientFactory`, `IConnector`, `Protocol`, `ClientCreator`. 
     77 * How to make concurrent connections -- it should explicitly state that: 
     78    * `connectTCP` (and other variations like `ClientCreator`, `connector.connect()`) 
     79      don't block -- they start the process of establishing the connection, and 
     80      you wait for an event handler like `connectionMade` to be called. 
     81    * you can simply call `connectTCP` (or whatever) multiple times -- in a row, 
     82      even -- to make multiple connections.  It's obvious once you "get it", but 
     83      needs to be explicitly stated, as it's not obvious to many people new to 
     84      Twisted. 
     85 
     86This document could be supplemented by links to these existing follow-up ("now that you know X you can try Y") documents: 
     87 
     88 * GUI docs (because client programs are often interactive) 
     89   * Choosing a reactor and GUI toolkit integration 
     90   * examples: `qtdemo.py`, `pyuidemo.py`, `wxdemo.py` 
     91 * `stdindemo.py` (again, interactivity) 
     92 * Reactor basics 
     93 * Logging howto 
     94 
     95This document could be supplemented by as-yet non-existant pre-requisite ("you should read this first") documents on: 
     96 
     97 * "Mastering TCP in under 3 minutes" ;) 
     98 
     99This document could be supplemented by links to these as-yet non-existant follow-up ("now that you know X you can try Y") documents: 
     100 
     101 * comprehensive description of `twisted.internet.protocol.Protocol`, the methods you can 
     102   override, and the actions you can perfrom (e.g. 
     103   `self.transport.loseConnection()`).  i.e. "Guide to writing protocols" 
     104 * Guide to `twisted.protocols.basic` 
     105 * Guide to `twisted.words.protocols.irc` (for better understanding of the IRC 
     106   example). 
     107 * Guide to `twisted.internet.stdio` (many client programs are likely to be 
     108   interactive). 
     109 * `gtkdemo.py` (we seem to have one for every other GUI...) 
     110 
     111== Style == 
     112 
     113The following changes to the style of the document would make it easier to read: 
     114 
     115 * typo: s/decended/descended/ 
     116 * it starts with a pretty jargon-heavy introduction.  I think a simple example 
     117   that is then dissected would be a better introduction. 
     118 * "is usually where set up" -> "is where set up usually" 
     119 * s/protocl/protocol/ 
     120 * first sentence of `ClientFactory` is "We use `reactor.connect*` and a 
     121   `ClientFactory`."  The sentence doesn't really make sense... use it to do what? 
     122   I'd expect some sort of explanation about why you'd want to use it, e.g. 
     123   "`ClientFactory` can be subclassed to XXX." 
     124 * "Persistent Data in the Factory" appears to be presenting an extract of the 
     125   previous example (to highlight certain things), but it's not clear that this 
     126   is an extract rather than a new example. 
     127 
     128== Overall summary == 
     129 
     130This document is: better than having no documentation, but with some glaringly obvious improvements to make 
     131