[Twisted-Python] Re: [Twisted-commits] r16552 - fix some docstrings and radix's nitpicking

[Jean-Paul Calderone]

On Wed, 05 Apr 2006 21:09:06 -0600, David Reid <dreid at wolfwood.twistedmatrix.com> wrote:
>Author: dreid
>Date: Wed Apr  5 21:09:05 2006
>New Revision: 16552
>
> [snip]
>
>Modified: branches/http-auth-1475-2/twisted/web2/auth/digest.py
>==============================================================================
>--- branches/http-auth-1475-2/twisted/web2/auth/digest.py	(original)
>+++ branches/http-auth-1475-2/twisted/web2/auth/digest.py	Wed Apr  5 21:09:05 2006
>@@ -23,7 +23,7 @@
>     'sha': sha.sha,
> }
>
>-def DigestCalcHA1(
>+def calcHA1(
> [snip]
>-def DigestCalcResponse(
>+def calcResponse(

You should leave the original function names around here somewhere, in a comment or a doc-string, since they are direct references to the spec which defines these transformations.

>
> [snip]
>
>Modified: branches/http-auth-1475-2/twisted/web2/auth/interfaces.py
>==============================================================================
>--- branches/http-auth-1475-2/twisted/web2/auth/interfaces.py	(original)
>+++ branches/http-auth-1475-2/twisted/web2/auth/interfaces.py	Wed Apr  5 21:09:05 2006
> [snip]
>
>     def decode(response, method=None):
>         """Create a credentials object from the given response.
>-
>+        May raise twisted.cred.error.LoginFailed if the response is invalid.
>+

You want @raise here.

>         @type response: C{str}
>         @param response: scheme specific response string
>
>
>Modified: branches/http-auth-1475-2/twisted/web2/auth/wrapper.py
>==============================================================================
>--- branches/http-auth-1475-2/twisted/web2/auth/wrapper.py	(original)
>+++ branches/http-auth-1475-2/twisted/web2/auth/wrapper.py	Wed Apr  5 21:09:05 2006
> [snip]
>@@ -51,10 +51,9 @@
>                                 from locateChild and render upon successful
>                                 authentication.
>
>-        @param credentialFactory: An L{ICredentialFactory} that supports the
>-                                  desired authentication scheme (currently only
>-                                  a single authentication scheme at a time is
>-                                  supported)
>+        @param credentialFactories: A list of instances that implement
>+                                    L{ICredentialFactory}.

"instances that implement"?  This is confusing.  Do you really mean "classes which implement" or "instances which provide"?

>+        @param credentialFactories: L{list}

This is probably meant to be @type.

>
>         @param protal: the portal that handles login

Typo here, and the only thing portals do is handle login, so is this really a useful piece of documentation? ;)

Jean-Paul