Code review comment for lp:~numerigraphe/openobject-server/trunk-partner-category-short-name

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

Hi Lionel,

I agree with the idea of making the name_get of Partner Categories configurable with the context, pretty much like what we already do with Partner Addresses.

However, in order to be consistent I'd suggest using a similar convention for the context keys to what we have for addresses:
    {'partner_category_display': 'short'}

Also, wouldn't it be better to delegate the name_get to super() as soon as the short format is enabled, rather than making the current code more complex?

Finally, as for turning this on the default partner action, I'd suggest keeping this in extra modules where it's used, because this would have no visible effect for the standard addons (category m2m and list display the full category name anyway), so it makes little sense, so it could be removed during a future cleanup anyway.

Instead of asking you to fix the above I did it on the fly while merging, I hope you won't mind ;-) I also adapted the docstring to use our new preferred convention for documenting context options.
Your idea has been preserved and you should be able to use it as you planned in extra modules - and it's credited to you of course.

Thanks for your excellent contributions, as usual!

review: Approve (tech details to be fixed while merging)

« Back to merge proposal