[Zope-dev] Re: [Zope] Re: Give 'Extensions' a configurable directory in zope.conf

[Rodrigo Dias Arruda Senra]

[ Florent Guillaume <fg at nuxeo.com> ]:
--------------------------------------------------------
| > >    * /lib/python/Products/ExternalMethod/ExternalMethod.py
> > 
> >      - little typos (not related, but since I was there <wink>)
> 
> It's a good practice to fix typos and such in a separate patch/checkin,
> so that those reading the diffs or patches can concentrate on what's
> really done.

Makes perfect sense. I even thought about submitting a different patch for
the typos. But then again, it seemed silly to submit a patch for two typos.
So I decided to wrap it up all in one single patch. But I can do it the
proper way next time.

> It's also nice to use modern python coding style (even when copy/pasting
> code):

Sure, I'll copy/modernize/paste from now on <wink>.

> >  That was the last statement in getPath(). If r==None then getPath()
> >  wouldn't return None all the same ?
> 
> It's in a loop. The first working path is returned.

Crystal clear,  merci beaucoup.

Should I re-submit the patch after applying your suggestions ...

 - modernize style
 - different patch for typos
 - supress 'if r is not None:' in the added code (unnecessary)

or the reviewer takes care of it ?


> But now that you put it in the collector it's ok. Thanks for the patch.

My pleasure.

best regards,
Senra