Code review comment for lp:~adiroiban/launchpad/bug-525371

Revision history for this message
Данило Шеган (danilo) wrote :

Hi Adi,

У чет, 20. 05 2010. у 19:06 +0000, Adi Roiban пише:
> > This is not needed for me, because just putting it as
> >
> > translation_messages = Attribute(_(
> > 'All `ITranslationMessage` objects related to this translation file.'
> > ))
> >
> > works fine. Not that there's anything wrong with your approach, I am just noting this.
>
> This will break the styleguide as there is a space before the ')'

Heh, this is not really a space: it's a multiline wrapping of braces,
and we do exactly the same thing with tuples (we'd do it with imports as
well if we didn't have too many of those), as mentioned on [1]:

  something = (
      FirstVeryLongName,
      SecondVeryLongName,
  )

[1] https://dev.launchpad.net/PythonStyleGuide#Multiline%20braces

Anyway, as I said, your style is just as fine as well.

 review approve
 merge approve

I'll get this into ec2 land soon.

Cheers,
Danilo

review: Approve

« Back to merge proposal