openerp-community-reviewer team mailing list archive
-
openerp-community-reviewer team
-
Mailing list archive
-
Message #00586
Re: [Merge] lp:~elicoidal/multi-company/multi-company into lp:multi-company
Review: Needs Fixing code review, no tests
Hi Eric,
This module sounds very promizing, but it'll really need time to review as it is a big one ! Before going into details, I first suggest to clean a bit the code:
- Avoid using SQL directly and use the ORM instead (quite lots of places)
- Some method are very long here, some refactoring would be very appreciated (e.g. 469, 769, 1572,...)
I would also personally loved to have some better explanation in the description of the module and help tooltip of fields. As this may lead to a very complexe behaviour in OpenERP, better to have good docs on how it works in case of cancellation or partial docs for example.
Would you think you'll have time time do this ?
Honestly, this is a very needed module, so I think it worth the pain !
Thanks in advance,
Regards,
Joël
--
https://code.launchpad.net/~elicoidal/multi-company/multi-company/+merge/179623
Your team Multi Company Core Editors is subscribed to branch lp:multi-company.
Follow ups