Code review comment for lp:~jml/quickly/template-cleanup

On Thu, Dec 31, 2009 at 7:53 PM, Didier Roche <email address hidden> wrote:
> Review: Approve
> Hey Jonathan, thanks for your path.
>
> I really have to find a vim script to format PEP8 compliant a python code (I know, you use emacs and don't care ;))
>

Heh heh.

> Thanks for this part:
> 647     - if __python_name_data_directory__.startswith('/'):
> 648     - pathname = __python_name_data_directory__
> 649     - else:
> 650     - pathname = os.path.dirname(__file__) + '/' + __python_name_data_directory__
> 651     + # Get pathname absolute or relative.
> 652     + path = os.path.join(
> 653     + os.path.dirname(__file__), __python_name_data_directory__)
> I didn't know that joining two absolute path only take the second. Handy trick :)
>
> Just a question, what is the __all__ in python_nameconfig.py is for? I only know this for autoimporting module in __init__.py, and you seem to still import function one by one:
> from python_name.python_nameconfig import get_data_file
> So, I don't really see its usefulness ;)
>

Ahh OK. It's there to indicate which names in the module are really
public. We don't have to have it if you don't want. Launchpad uses it
to check that people aren't using private stuff (we have a utility
called "importfascist" that does this).

> also, why importing multiple module in a same line is wrong?
> (cf commit 404)
>

PEP 8 says so.

> BTW, merged, thanks! I'll try to go file by file to include PEP8 compliance, but this is a long term work :)
>

Going file-by-file is the best approach. I changed all of these
because I wanted to start a Quickly project and not have a bunch of
red warnings on my screen. :)

jml