openerp-community-reviewer team mailing list archive

["Eric Caudal - www.elico-corp.com" <eric.caudal@xxxxxxxxxxxxxx>]

Hi Joël,
Thanks for the review but I think you can actually scrap this merge...

Indeed, in the meantime, the module has been completely rewritten with a 
new version 2 which is much cleaner and works now for SO/PO (SP/SM being 
now under development).

Augustin is gonna  propose the merge soon as it works now quite smoothly.

Behavior is based on the following blueprint which could be the base 
documentation for the 
module:https://blueprints.launchpad.net/multi-company/+spec/icops
But I will work on some additional simple setup case as this is quite 
complex indeed (actually the new module comes with a complete unit test 
and demo set!).

Version 3 will be based on the Magento Connector as we discussed time ago...

Eric CAUDAL

Eric Caudal
/CEO/
--
*Elico Corporation, Shanghai branch
/OpenERP Premium Certified Training Partner/  *
Cell: + 86 186 2136 1670
Office: + 86 21 6211 8017/27/37
Skype: elico.corp
eric.caudal@xxxxxxxxxxxxxx  <mailto:eric.caudal@xxxxxxxxxxxxxx>
http://www.elico-corp.com

Elico Corp
On 16/10/2013 18:03, Joël Grand-Guillaume @ camptocamp wrote:
> 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.