Code review comment for lp:~itohm/nova/enable-rightaws

Revision history for this message
Masanori Itoh (itohm) wrote :

Hi Rick,

Thanks for the review!

I updated the branch according to your comments.
Actually, I found some more lines which need I18N treatment
in the same method which I modified and fixed them.

Also, I rebased to trunk rev 987, ran run_tests.sh and did some other checking.
I hope this looks goot for folks.

Regards,
Masanori

From: Rick Harris <email address hidden>
Subject: Re: [Merge] lp:~itoumsn/nova/enable-rightaws into lp:nova
Date: Thu, 14 Apr 2011 05:55:46 +0900

> Review: Needs Fixing
> Looks good, nice work Masanori.
>
> Just some *really* small style points:
>
> > 4 + LOG.debug('host_only_signature: %s', host_only_signature)
>
> Needs i18n treatment.
>
> > 50 + [address, sep, port] = server_str.partition(':')
>
> Might be cleaner to use `split` since the separator is ignored as well as unpacking to a tuple since that's more conventional:
>
> (address, port) = server_str.split(':')
>
> Side note: I think when/if we move to 2.7 we can eventually replace the host-extraction code with urlparse (which supports IPv6 addresses and differentiating the host and port portion of the netloc).
>
>
>
> --
> https://code.launchpad.net/~itoumsn/nova/enable-rightaws/+merge/57370
> You are the owner of lp:~itoumsn/nova/enable-rightaws.

« Back to merge proposal