genesis-devs team mailing list archive

[Frederik Elwert <frederik.elwert@xxxxxx>]

Hi!

Am Montag, den 21.12.2009, 14:03 +0100 schrieb David Castellanos: 
> >      * Why did you add `genesis.pot` to `locale/templates`? It used to
> >        be unter `po`. Has this a specific advantage? I think the
> >        update_translations script relies on the po template to be in
> >        the po directory.
> 
> I had no idea, I didn't find that file anywhere, neither that script
> :-S. I thought it would be a good idea to add it to the source code
> distribution, for those who download the source from launchpad but
> doesn't have an account, or doesn't know how lp works.
> 
> But feel free to remove it or change its location, without problem :-)

I removed it, because the one in po/ should be enough. I hope it hasn’t
any issues that you fixed in the other version.

> >      * Is it necessary to use that complicated
> >        subprocess.Popen().communicate()[0] construct in setup.py? Isn’t
> >        a simple subprocess.call() sufficient?
> 
> To be honest, I've found that code snippet in google.com/codesearch.
> It seems to work and I didn't think in refactoring it, firstly because
> I'm lazy and I had little experience with thread management in Python
> :-S, I'm sorry.

I was just curious, if that Popen version has any benefits. E.g., you
can’t access stdout when using sp.call(). But since I think we don’t
need that, I changed it to a simple sp.call().

On Google code search, I found both variants, so neither seems to be
wrong. :-)

> Please take a look at it, and perform all the changes you consider
> necessary. Sorry for the inconvenience that I may have caused.

No inconvenience at all, that fixes were really helpful. I just try to
practice code reviews, and these were the only bits I stubled upon. Both
were no real problems.

> Currently I'm very busy, but the following weekend I'll try to take a
> look at it :-S

No hurry. :-)

Have nice holydays!

Cheers,
Frederik