Review: Disapprove

Hi all, sorry for the late replay, but I'm having hard days...

Although the convergence of both modules is desired, I have also to disagree 
technically with this MP because:

- It makes a direct replacing of the field city, which breaks automatically any 
existing installation with data in city field, because there are string in the 
DB on that field, and with this module, it is expected to exist an ID (integer).
- It is incompatible with other modules that expect to have a string on that 
field. For example, CRM.
- It doesn't provide a menu entry for managing cities.
- It is not adapted to v7 conventions.

So, please let me insist on trying to get base_location working for all. There 
are two requirements to be met for your approach:

- Removing the requirement of the zip field. City field is still required. We 
have to change a little 'name_get' method and add a 'search_name' method to 
fulfill complete ORM compatibility.
- As Raphaël, I also think that making city related to state is better than to 
country, but if you still need it, we can add a field country on city model 
that can be automatically filled when you enter the state (and also made 
readonly), and modify partner/company on_change accordingly.

What do you think about this proposal?

Regards.
-- 
https://code.launchpad.net/~savoirfairelinux-openerp/partner-contact-management/city-move/+merge/196023
Your team Savoir-faire Linux' OpenERP is subscribed to branch 
lp:~savoirfairelinux-openerp/partner-contact-management/city-move.

-- 
Mailing list: https://launchpad.net/~savoirfairelinux-openerp
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~savoirfairelinux-openerp
More help   : https://help.launchpad.net/ListHelp

Reply via email to