Code review comment for lp:~matsubara/launchpad/bug-628510-oops-permission

Revision history for this message
Diogo Matsubara (matsubara) wrote :

On Fri, Oct 8, 2010 at 7:07 PM, Gary Poster <email address hidden> wrote:
> Review: Approve
> merge-conditional
>
> Great, thank you Diogo.
>
> 1) Please run make lint and make sure that you respond to its concerns (though note that it has one error I'm aware of: tuples with a single item such as (1,) should be rendered without a trailing space after the comma, so "(1,)" is preferred over "(1, )").

Fixed make lint warnings.

>
> 2) You have indentation that's a bit non-standard in uniquefileallocator.py and test_uniquefileallocator.py.  Here's an example.
>
>        wanted_permission = (stat.S_IRWXU | stat.S_IRGRP | stat.S_IXGRP |
>            stat.S_IROTH | stat.S_IXOTH)
>
> I'd prefer to see that as follows:
>
>        wanted_permission = (stat.S_IRWXU | stat.S_IRGRP | stat.S_IXGRP |
>                             stat.S_IROTH | stat.S_IXOTH)
>
> This is another acceptable alternate:
>
>        wanted_permission = (
>            stat.S_IRWXU | stat.S_IRGRP | stat.S_IXGRP | stat.S_IROTH |
>            stat.S_IXOTH)

Fixed the identation.

>
> That's it.  Thank you!
>

Thanks!
--
Diogo M. Matsubara

« Back to merge proposal