[Zope-dev] Re: [Zope] Re: Give 'Extensions' a configurable directory in zope.conf
* /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. * /lib/python/App/Extensions.py - in getPath() check if directive 'extensions' is overriden in zope.conf, and use it instead. The relevant code added to getPath() is depicted below. code if (prefix==Extensions) and (cfg.extensions is not None): r=_getPath(cfg.extensions, '', name, suffixes) if r is not None: return r /code It's also nice to use modern python coding style (even when copy/pasting code): if prefix == Extensions and cfg.extensions is not None: r = _getPath(cfg.extensions, '', name, suffixes) if r is not None: return r However, I did not understood the purpose of the construction if r is not None: return r. 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. I believe this is the last bit of doubt before submitting the patch. BTW, maybe I should have posted this to Zope-Dev instead ? Yep :) I Cc zope-dev, and followup there. But now that you put it in the collector it's ok. Thanks for the patch. Florent -- Florent Guillaume, Nuxeo (Paris, France) CTO, Director of RD +33 1 40 33 71 59 http://nuxeo.com [EMAIL PROTECTED] ___ Zope-Dev maillist - [EMAIL PROTECTED] http://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://mail.zope.org/mailman/listinfo/zope-announce http://mail.zope.org/mailman/listinfo/zope )
[Zope-dev] Re: [Zope] Re: Give 'Extensions' a configurable directory in zope.conf
[ Florent Guillaume [EMAIL PROTECTED] ]: | * /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 ___ Zope-Dev maillist - [EMAIL PROTECTED] http://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://mail.zope.org/mailman/listinfo/zope-announce http://mail.zope.org/mailman/listinfo/zope )
Re: [Zope-dev] Re: [Zope] Re: Give 'Extensions' a configurable directory in zope.conf
Should I re-submit the patch after applying your suggestions ... No need. Florent -- Florent Guillaume, Nuxeo (Paris, France) CTO, Director of RD +33 1 40 33 71 59 http://nuxeo.com [EMAIL PROTECTED] ___ Zope-Dev maillist - [EMAIL PROTECTED] http://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://mail.zope.org/mailman/listinfo/zope-announce http://mail.zope.org/mailman/listinfo/zope )
