Code review comment for lp:~gabor-karsay/awn-extras/various-fixes

And make sure the code is PEP8 compliant.

On Tue, Sep 14, 2010 at 10:47 PM, onox <email address hidden> wrote:

> Gabor, it seems the linked bug reports on this page are different from
> those in your description. Could you fix this? Please try to avoid reusing
> some branch for different things. (It's good, however, that you use
> branches, allows people to review the code :)) Furthermore, about those bugs
> you described, set them to "Confirmed" once you can reproduce them. Their
> status still seems to be "New". Use "In Progress" if you're working on them.
>
> About the milestones: some people set bugs to milestone 0.4.2 if the bug
> occurs in a previous version and will be fixed in 0.4.2. Personally I use
> 0.4.2 whenever the fix ends up in 0.4.2, this including bugs that were
> introduced in between 0.4.0 and 0.4.2 (0.4.1 / trunk)
>
> About the code: I haven't tested it (because I don't (want) / can't use the
> mail applet) so I don't know whether the changes are functional correct. One
> thing you could do, however, is change def unlock to:
>
> if not info.get_is_locked():
> return True
>
> # here the indented code
>
> return not info.get_is_locked()
>
> This way, the most important piece of code of the function is 1 indention
> less. One could argue it's a little big more readable.
> Oh, and you could replace the url to "launchpad bug #432882"
> --
>
> https://code.launchpad.net/~gabor-karsay/awn-extras/various-fixes/+merge/35450
> You are requested to review the proposed merge of
> lp:~gabor-karsay/awn-extras/various-fixes into lp:awn-extras.
>