Code review comment for lp:~openerp/openobject-server/web-dashboard

Revision history for this message
Olivier Dony (Odoo) (odo-openerp) wrote :

Some additional feedback:

- why are you using type='char' for the *.datas fields - these should be binary fields, right? Have you ensured to be compatible with the current fields.binary implementation that allows reading size instead of content of binary fields. We don't want GTK to be reading these, do we?
- please rename the *datas field - data is already a plural form - "datas" does not exist (yes I know we have many typos like that already.. so let's not introduce more!)
- why do you do the resolution of the "web_icon*" paths during menu creation in convert.py? What if I want to move my addons after installing the module? Please do it in the function fields.
- final note: if these values are accessed often, you would do good to make the function fields "stored", so the icons can be cached (user can simply save the menu again to refresh it) And when you do that, you really need to make them type='binary' and conform to the binary logic

Thanks!

review: Needs Fixing

« Back to merge proposal