On Nov 17, 2012, at 5:56 AM, Cédric Krier <[email protected]> wrote:

> Hi,
> 
> I have made a quick review of nereid in foresight of his inclusion.

Thanks for taking the time to review nereid

> So here are some comments:
> 
> * Split into two packages:
> 
>    - nereid: the flask application
>    - trytond_nereid: the Tryton module

Agree, and package nereid depends on trytond_nereid ?

> 
> * Versioning: we should switch to the same versioning schema of Tryton

Agree

> 
> * Upgrade to trunk (Active Record)

Agree, so for older version we leave it as such on github ?

> 
> * Use similar COPYRIGHT method

Agree

> 
> * MemcachedSessionStore should be rename as it is multi-backend support

Agree

> 
> * render_email should be moved to trytond base

render_email depends on the template loader and the render_template method 
which depends on nereid/flask

> 
> * the jinja2 cache should be configurable instead of forcing Memcache
> 
> * I would drop TrytonTemplateLoader to use only FileSystemLoader

Agree, it is only used for testing anyway.

> 
> * I would drop crontrib.gravatar because not really required and could be
>  provided by external lib

Agree

> 
> * also contrib.sphinxsearch

Agree

> 
> * and contrib.testing.xmlrunner

Agree

> 
> * Not sure ir.attachment.uploaded_by is really needed

There is no way to know the nereid user who uploaded the document without
this field

> 
> * There is a misunderstanding about Address because it is used as a
>  user, it should be move to party.

This has been resolved in recent version with nereid.user object. It is the
doc string which has a bug. And nereid.user _inherits from party.party

> 
> * The dependency to ccy could be dropped or at least move into a
>  separate module

I am in for dropping this. I am not fond of the library either and its
poorly maintained. What is your recommendation for linking
currency with locale ? (in a separate module of course)

> 
> * Test framework should be adapted to trytond one.

It uses unittest2 (or unittest in python 2.7). Do you want this
dependency to be dropped ?

> 
> 
> Remarques:
> 
> - I'm not sure to understand: nereid/caching.py

The Cache class is a wrapper over the current_app's cache instance.
This wrapper pattern allows the method 'cache' of the class to be used
as a decorator (like memoize) even before there is a cache instance.

> 
> - I'm wondering of Form could not be generated automaticaly.

I am working on this, but I have not made much progress so far.

> 
> -- 
> Cédric Krier
> 

Thanks,

Sharoon Thomas
Openlabs Technologies & Consulting (P) Limited

w: http://www.openlabs.co.in
m: +1 813.793.6736 (OPEN) Extn. 200
t: @sharoonthomas 

- We CARE for our customers

-- 
-- 
[email protected] mailing list



Reply via email to