> On Mon, 2009-08-17 at 13:00 +0000, Duncan McGreggor wrote:
> >
> > +from txaws.util import XML, calculate_md5
> >  from txaws.credentials import AWSCredentials
> > -from txaws.util import XML
> 
> PEP8 mandates sorting this:
> from txaws.credentials should be the first line.

[1] 

So PEP-8 states the following:

      1. standard library imports
      2. related third party imports
      3. local application/library specific imports

Both lines are local application, and there's no further specification on 
sorting those, is there? Or am I missing something?

> > +
> > +
> > +NS = '{http://s3.amazonaws.com/doc/2006-03-01/}'
> 
> Rather than NS, perhaps name_space.

[2]

Good idea -- done.
 
> >  class S3Request(object):
> > -    def __init__(self, verb, bucket=None, objectName=None, data='',
> > -            contentType=None, metadata={},
> > rootURI='https://s3.amazonaws.com',
> > -            creds=None):
> > +
> > +    def __init__(
> > +        self, verb, bucket=None, object_name=None, data='',
> > content_type=None,
> > +        metadata={}, root_uri='https://s3.amazonaws.com',
> > creds=None):
> 
> I believe PEP-8 allows the style of wrapping that was in use before - I
> won't insist either way, but I do fine using as much of the line as
> possible nicer. e.g
>
> def __init__(self, verb, bucket, object_name, data,
>     content_type, metadata):

[3]

I don't think that's PEP-8, but I may be misreading the PEP. If you take a look 
at the section that starts with "The preferred way of wrapping long lines..." 
and the example code below that, they wrap to the point where the parens of the 
contained code starts, not a 4-space indent (I was actually only made aware of 
this last year).

I compromised, and re-wrapped like this:

    def __init__(self, verb, bucket, object_name, data,
                 content_type, metadata):

> > === modified file 'txaws/storage/test/test_client.py'
> > --- txaws/storage/test/test_client.py   2009-04-26 08:32:36 +0000
> > +++ txaws/storage/test/test_client.py   2009-08-17 12:49:40 +0000
> > @@ -4,17 +4,21 @@
> >
> >  from twisted.internet.defer import succeed
> 
> c,s,t,u - sorting :) on the import lines below.
> 
> > +from txaws.util import calculate_md5
> > +from txaws.tests import TXAWSTestCase
> >  from txaws.credentials import AWSCredentials
> > -from txaws.tests import TXAWSTestCase
> > -from txaws.storage.client import S3, S3Request, calculateMD5
> > +from txaws.storage.client import S3, S3Request
> > +

[4] 

Again, same question about the ordering... can you be more specific about what 
part of PEP-8 I'm missing? Thanks!

> > === modified file 'txaws/util.py'
> > --- txaws/util.py       2009-04-27 08:53:11 +0000
> > +++ txaws/util.py       2009-08-17 12:49:40 +0000
> > @@ -6,16 +6,23 @@
> >
> >  __all__ = ['hmac_sha1', 'iso8601time']
> 
> and here too.
> 
> > +import time
> > +import hmac
> > +from hashlib import sha1, md5
> >  from base64 import b64encode
> > -from hashlib import sha1
> > -import hmac
> > -import time
> > +
> 
> 
>  review needsfixing

[5] 

Ditto ;-)
-- 
https://code.edge.launchpad.net/~oubiwann/txaws/413738-method-name-cleanup/+merge/10245
Your team txAWS Team is subscribed to branch lp:txaws.

_______________________________________________
Mailing list: https://launchpad.net/~txawsteam
Post to     : txawsteam@lists.launchpad.net
Unsubscribe : https://launchpad.net/~txawsteam
More help   : https://help.launchpad.net/ListHelp

Reply via email to