← Back to team overview

openerp-community-reviewer team mailing list archive

Re: [Merge] lp:~trobz/web-addons/web-unleashed into lp:web-addons

 

Michel, thanks for your comments. I already doubted my programming skills when I couldn't find out where the context is propagated, good to hear it's not.

On context: You definitely need to think of a way to pass the user's standard context (openerp.session.user_context) to your models. Otherwise, you lose stuff like the timezone (which causes all functions dealing with datetimes to assume UTC, and you really don't want that) and default values. That's what currently comes to my mind, there are also other things like the currently active project or the like.

On evaluating context: It can contain code line time(), so if you eval it on view initialization, you'll always pass the same time.

Translations: The messages in the Errors you throw are not translated to my understanding. Even though most of them look like they never should occur in a production environment, I still think it's good practice to translate them.

Tests: I don't see real world disadvantages, so if you've reason to keep them there, keep them
-- 
https://code.launchpad.net/~trobz/web-addons/web-unleashed/+merge/195542
Your team Web-Addons Core Editors is requested to review the proposed merge of lp:~trobz/web-addons/web-unleashed into lp:web-addons.


Follow ups

References