Changes between Version 2 and Version 3 of DocumentationAnalysis/Testing/JonathanLange


Ignore:
Timestamp:
03/14/2006 11:44:29 PM (9 years ago)
Author:
jml
Comment:

--

Legend:

Unmodified
Added
Removed
Modified
  • DocumentationAnalysis/Testing/JonathanLange

    v2 v3  
    1  
    21= Review details = 
    32 
     
    2726= Document review = 
    2827 
    29 ''Have a look at wiki:DocumentationAnalysis/UserReviewTemplate for an alternative review style you might prefer.'' 
    30  
    3128== Coverage of subject matter == 
    3229 
    33 ''Documents should demonstrate and recommend only '''best practice''', except in clearly marked examples of bad code which leads to a justification of best practice. Any out-of-date or questionably intelligent ways of doing things shown in our documentation are problems. Best practice should be exhibited even when it makes examples longer.''  
    34  
    35 '''''Best practice''' includes: code doing what it puports to do; following Twisted coding style; using accepted idioms; and unless the documentation is to demonstrating a lower level feature by reimplementing a higher level one it should use appropriate library functions (eg LineReceiver) instead of doing a half-baked reimplementation. Further, code should be '''easily readable''' at the expense of lines of code -- code is more readable when the number of things it does per line is limited.'' 
    36  
    3730This document seems to cover the following subject matter at least acceptably well: 
    38 ''list here'' 
     31 * Don't use reactor.start, reactor.stop, reactor.iterate 
    3932 
    4033This document seems to be attempting to cover the following subject matter, but its coverage is flawed: 
    41  * ''example 3 puports to demonstrate a webserver, but does not in fact run'' 
    42  * ''section 2 says that the design of X solves Y problem when it actually doesn't'' 
    43  * ''example 7 is using a sequence of functions to get Y result when we actually provide a convienience function Z'' 
     34 * The timeout description doesn't describe what a timeout does to the test results 
     35 * The heading "Twisted-specific quirks: ..." shouldn't enumerate the quirks, and probably shouldn't call them quirks 
     36 * Discusses how to clean up the reactor but doesn't mention that leaving the reactor unclean will fail the test 
     37 * Should provide examples of how to clean up the reactor 
     38 * Explains that Trial extends unittest, but doesn't tease out what that means for writing tests 
    4439 
    4540This document ought to be covering the following subject matter but is not: 
    46  * ''the reason why Y is a better design than Z in this case'' 
    47  * ''Foo useful library function'' 
    48  * ''the X class of problems for which this approach needs to be modified'' 
     41 * the todo attribute 
     42   * its existence 
     43   * appropriate values to set it to 
     44   * why one might want to use it 
     45 * skipping tests 
     46   * that it is possible 
     47   * how to do it with the skip attribute 
     48   * how to do it by raising SkipTest 
     49 * That calling log.err will fail a test 
     50   * [although the api surrounding this is still under discussion!] 
    4951 
    50 This document is recommending the following things that it shouldn't be recommending: ''[recall that merely mentioning something in a document often counts as a recommendation, and mentioning something only to denounce it is confusing and should be limited]'': 
    51  * ''X out-of-date library function'' 
    52  * ''Y sequence of calls which have been superceded by Z library'' 
     52 
     53This document is recommending the following things that it shouldn't be recommending: 
     54 * Maybe mentioning reactor.iterate et al is a bad idea 
    5355 
    5456This document could be supplemented by links to these existing pre-requisitie ("you should read this first") documents: 
    55 ''list here'' 
     57 * some sort of document on how to write unit tests 
     58   * Python's unittest documentation? 
     59   * diveintopython.org has an excellent chapter on test-driven development 
     60 * The Trial manpage 
    5661 
    57 This document could be supplemented by links to these existing follow-up ("now that you know X you can try Y") documents: 
    58 ''list here'' 
     62This document could be supplemented by links to these existing follow-up ("now that you know X you can try Y") documents:  
    5963 
    6064This document could be supplemented by as-yet non-existant pre-requisite ("you should read this first") documents on: 
    61 ''list here'' 
    6265 
    6366This document could be supplemented by links to these as-yet non-existant follow-up ("now that you know X you can try Y") documents: 
    64 ''list here'' 
     67 * How to extend Trial 
     68 
    6569 
    6670== Style == 
    6771The following changes to the style of the document would make it easier to read: 
    6872 
    69  * ''shorter paragraphs in section 4'' 
    70  * ''less hyperbole about the Twisted Way in the introduction'' 
     73 * Way more examples 
     74 * Structure the entire document around how to test code, rather than documenting quirks 
     75 * Create headings within "Leave the Reactor as you found it" section 
     76 
    7177 
    7278== Overall summary == 
    7379 
    74 This document is: ''excellent / good, with some minor improvements to recommend / reasonable, with some pretty obvious improvements to make / better than having no documentation, but with some glaringly obvious improvements to make / so bad that it would be better to have no documentation on this'' 
     80The document seems particularly slanted toward writing tests to be ''contributed'' to Twisted, rather than writing tests for code that ''uses'' Twisted.  I recommend that the idiosyncracies of code destined for the Twisted repository be documented in the coding standards. 
     81 
     82This document is: better than having no documentation, but with some glaringly obvious improvements to make