Review: Needs Fixing code review and test

Hi, Joël and Romain,

Thank you very much for the effort you are making to get this module in the 
best way. I have tested it and view the code, and these are my remarks:

- We agreed about renaming to product_price_historization_by_company or it can 
be only product_price_historization, are we?
- You can put in the description that is multi-company aware and avoid it on 
the module name.
- Put product_multi_company_purchase_demo.yml on test folder and add it to 
tests on manifest file.
- Create at least the pot translation template file.
- Rename 'price.history' model to 'product.price.history' to be consistent.
- In security/ir.model.access.csv, there are references to groups installed by 
"mrp", "sale" or "hr"modules, but these modules are not declared as 
dependences. BTW, it doesn't make any sense to have it, because you already 
have enough permissions with groups like base.group_sale_manager or 
base.group_user.
- It would be very interesting to have any screen to query past prices.

Regards.
-- 
https://code.launchpad.net/~camptocamp/openerp-product-attributes/port-add-product_multi_company_7.0-bis-jge/+merge/192872
Your team OpenERP Community is subscribed to branch 
lp:openerp-product-attributes.

_______________________________________________
Mailing list: https://launchpad.net/~openerp-community
Post to     : openerp-community@lists.launchpad.net
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp

Reply via email to