Review: Disapprove

@Pedro,

I agree with your 4 objections to the current merge:
1) 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).
2) It is incompatible with other modules that expect to have a string on that 
field. For example, CRM.
3) It doesn't provide a menu entry for managing cities.
4) It is not adapted to v7 conventions.

As for 1), notice that in our localization we opted for creating a new key 
instead 
https://github.com/openerpbrasil/l10n_br_core/blob/7.0/l10n_br_base/res_partner.py#L73

Also, for compatibility, we set the partner.city char field properly when we 
select a city in the m2o 
https://github.com/openerpbrasil/l10n_br_core/blob/7.0/l10n_br_base/res_partner.py#L173

As for incompatibilities, such as with the CRM module you mention, well yes 
indeed it requires extra extensions. This is what we do for instance in Brazil 
to get it working: 
https://github.com/openerpbrasil/l10n_br_core/blob/7.0/l10n_br_crm/crm_lead.py#L137

Not trivial to get it working across the codebase, but I wonder if there is 
anything simpler possible. And yes, in most countries getting OpenERP to work 
requires a ton of hacks, this is a sad reality.

As for having a city m2o country, well at least for us we don't need it. I 
think the problem might more be with Vauxoo and Savoir Faire who opted for a 
m2o to the country before I said I would need it to the state.

I'm waiting your proposal for base_location then. Do you have some existing MP? 
Because like the other said, in the current form, base_location doesn't seem to 
do the cut for us (no city object, and like Pierre said, we also have thousands 
of zips per city here).

@Maxime, right, France doesn't have states. But I guess that we could use 
department or regions here. In any case, with the current financial crisis, 
it's highly probably to see a total balkanization of the country collapsing 
under regional independentist forces just like in Spain or Belgium. Just 
saying... ;-p

Thank you all for your participation.
-- 
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