[11.0][MIG] base_location_nuts: Migration to 11.0 #518

agaldona · 2017-11-14T12:24:47Z

oihane · 2017-11-14T16:25:52Z

base_location_nuts/__manifest__.py

+    'category': 'Localisation/Europe',
+    'version': '11.0.1.0.0',
+    'depends': [
+        'sales_team',


I think that you can now remove this dependency. It was required for the menuitem.

oihane

LGTM installed on an enterprise db and no problem.

yajo · 2017-11-16T08:27:40Z

base_location_nuts/wizard/nuts_import.py

-        pattern = re.compile(r'^.*<\?xml', re.DOTALL)
-        content_fixed = re.sub(pattern, '<?xml', res_request.content)
-        if not re.match(r'<\?xml', content_fixed):
+        pattern = re.compile(r'^.*<\?xml'.encode('utf-8'), re.DOTALL)


just do rb'^.*<\?xml' instead of the encode stuff.

yajo · 2017-11-16T08:28:25Z

base_location_nuts/wizard/nuts_import.py

-        content_fixed = re.sub(pattern, '<?xml', res_request.content)
-        if not re.match(r'<\?xml', content_fixed):
+        pattern = re.compile(r'^.*<\?xml'.encode('utf-8'), re.DOTALL)
+        content_fixed = re.sub(pattern, b'<?xml',


It doesn't make much sense to manually compile a pattern that you're gonna use just once, even more when python already autocaches often-used pattern compilations.

Check explanation here: https://docs.python.org/3/library/re.html#re.compile

yajo · 2017-11-16T08:30:07Z

base_location_nuts/wizard/nuts_import.py

+        pattern = re.compile(r'^.*<\?xml'.encode('utf-8'), re.DOTALL)
+        content_fixed = re.sub(pattern, b'<?xml',
+                               res_request.content)
+        if not re.match(r'<\?xml'.encode('utf-8'), content_fixed):


Again, just rb'<\?xml'

yajo · 2017-11-16T08:30:33Z

base_location_nuts/wizard/nuts_import.py

@@ -190,7 +190,8 @@ def run_import(self):
        # All current NUTS (for available countries),
        #   delete if not found above
        nuts_to_delete = nuts_model.search(
-            [('country_id', 'in', [x.id for x in self._countries.values()])])
+            [('country_id', 'in',
+              [x.id for x in list(self._countries.values())])])


No need for list() here. Views are better when iterating.

The same about the automatism of 2to3

yajo · 2017-11-16T08:31:32Z

base_location_nuts/wizard/nuts_import.py

            raise UserError(_('Downloaded file is not a valid XML file'))
        return content_fixed

    @api.model
    def _load_countries(self):
-        for k in self._countries.keys():
+        for k in list(self._countries.keys()):


Replace with for k in self._countries:

This is done automatically by 2to3, and there are reasons for that:

self._countries doesn't return now the keys by default

It's converted to list because they don't get if it's going to be used as an iterator. In this case list( can be removed.

This is a combination of both things, but both things are wrong in this specific case AFAICS.

Check this out, from last PyCon: https://youtu.be/V5-JH23Vk0I?t=3m17s

yajo · 2017-11-16T09:11:56Z

base_location_nuts/wizard/nuts_import.py

            raise UserError(_('Downloaded file is not a valid XML file'))
        return content_fixed

    @api.model
    def _load_countries(self):
-        for k in list(self._countries.keys()):
+        for k in self._countries.keys():


Please remove .keys()

It's not finished yet... I have to take a look also to re.compile thing.

Haha OK, too quick review 😋

pedrobaeza · 2017-11-21T17:14:05Z

@luismontalba @chienandalu please review this one and l10n_es_location_nuts one.

chienandalu

Looks good. It'd be nice to improve coverage anyway 🙂

chienandalu · 2017-11-21T18:22:19Z

@pedrobaeza I can't see any PR for l10n_es_location_nuts, do you have a link?

pedrobaeza · 2017-11-21T18:26:53Z

OCA/l10n-spain#691

yajo · 2017-11-30T11:13:32Z

Please squash the migration commits to merge.

oihane · 2017-12-04T14:39:24Z

@yajo squash done

yajo · 2017-12-05T08:26:52Z

Yikes I forgot to ask you to squash translation commits too, I'll do it for you. 😉

[MIG] Rename manifest files

This makes NUTS labels and levels to be stored in the res.country object. Now creating l10n submodules is a piece of cake! Relational fields now follow guidelines on naming. Old name attribute used for backwards compatibility wherever needed. Also some methods have been renamed, and refactored to be smarter. Most cases l10n modules will just need to fill the res.contry table, and regions and substates domains will work out of the box. In case you still need to overwrite any method, splitting in smaller methods makes it easier too. Oh! And no need for recursive dictionary updates. Return dict() to make it easier for submodules to add domains. Fix KeyError: 'substate_id_level'.

Fix linter. Fix typo.

…except. (OCA#506)

yajo · 2017-12-05T08:53:12Z

Rebased squashing by author & logical changeset. If bots go ✔️, merge.

oihane · 2018-01-23T15:48:40Z

Can anyone check this out again, bots have gone green...

agaldona mentioned this pull request Nov 14, 2017

[11.0][MIG] l10n_es_location_nuts: Migration to 11.0 OCA/l10n-spain#691

Merged

1 task

agaldona force-pushed the 11.0-mig-base_location_nuts branch 3 times, most recently from 0593408 to ebdb16b Compare November 14, 2017 13:23

oihane reviewed Nov 14, 2017

View reviewed changes

agaldona force-pushed the 11.0-mig-base_location_nuts branch from ebdb16b to 787593e Compare November 14, 2017 16:42

oihane approved these changes Nov 14, 2017

View reviewed changes

agaldona force-pushed the 11.0-mig-base_location_nuts branch from 787593e to 562d983 Compare November 14, 2017 16:46

pedrobaeza added this to the 11.0 milestone Nov 14, 2017

pedrobaeza mentioned this pull request Nov 14, 2017

[11.0][MIG] base_location_nuts #483

Closed

oihane force-pushed the 11.0-mig-base_location_nuts branch 2 times, most recently from 852b863 to 77d6cfe Compare November 15, 2017 15:15

yajo requested changes Nov 16, 2017

View reviewed changes

oihane force-pushed the 11.0-mig-base_location_nuts branch from fc2740d to 9e18fe6 Compare November 16, 2017 13:59

pedrobaeza mentioned this pull request Nov 21, 2017

Migration to version 11.0 #482

Closed

53 tasks

chienandalu approved these changes Nov 21, 2017

View reviewed changes

luismontalba approved these changes Nov 22, 2017

View reviewed changes

yajo approved these changes Nov 30, 2017

View reviewed changes

oihane force-pushed the 11.0-mig-base_location_nuts branch from 9e18fe6 to 2d76bf3 Compare December 4, 2017 09:15

Antonio Espinosa and others added 4 commits December 5, 2017 09:51

[ADD] NUTS regions as a base addon

b289a4a

[MIG] Make modules uninstallable

c4f340f

[MIG] Rename manifest files

Use separate and explicit fields for each NUTS level

01ad0bf

yajo and others added 5 commits December 5, 2017 09:51

Make method recieve the level as int.

90c956d

Fix linter. Fix typo.

[MIG] base_location_nuts: Migration to 10.0

8676d5f

[FIX] base_location_nuts: Prevent flake8 error for non-critical bare …

dcee932

…except. (OCA#506)

OCA Transbot updated translations from Transifex

03f7470

[MIG] base_location_nuts: Migration to 11.0

e014100

yajo force-pushed the 11.0-mig-base_location_nuts branch from 2d76bf3 to e014100 Compare December 5, 2017 08:52

pedrobaeza merged commit 7eb6c73 into OCA:11.0 Jan 23, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[11.0][MIG] base_location_nuts: Migration to 11.0 #518

[11.0][MIG] base_location_nuts: Migration to 11.0 #518

agaldona commented Nov 14, 2017 •

edited

Loading

oihane Nov 14, 2017

oihane left a comment

yajo Nov 16, 2017

yajo Nov 16, 2017

yajo Nov 16, 2017

yajo Nov 16, 2017

pedrobaeza Nov 16, 2017

yajo Nov 16, 2017

pedrobaeza Nov 16, 2017

yajo Nov 16, 2017 •

edited

Loading

yajo Nov 16, 2017

oihane Nov 16, 2017

yajo Nov 16, 2017

pedrobaeza commented Nov 21, 2017

chienandalu left a comment

chienandalu commented Nov 21, 2017

pedrobaeza commented Nov 21, 2017

yajo commented Nov 30, 2017

oihane commented Dec 4, 2017

yajo commented Dec 5, 2017

yajo commented Dec 5, 2017

oihane commented Jan 23, 2018

[11.0][MIG] base_location_nuts: Migration to 11.0 #518

[11.0][MIG] base_location_nuts: Migration to 11.0 #518

Conversation

agaldona commented Nov 14, 2017 • edited Loading

Choose a reason for hiding this comment

oihane left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yajo Nov 16, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pedrobaeza commented Nov 21, 2017

chienandalu left a comment

Choose a reason for hiding this comment

chienandalu commented Nov 21, 2017

pedrobaeza commented Nov 21, 2017

yajo commented Nov 30, 2017

oihane commented Dec 4, 2017

yajo commented Dec 5, 2017

yajo commented Dec 5, 2017

oihane commented Jan 23, 2018

agaldona commented Nov 14, 2017 •

edited

Loading

yajo Nov 16, 2017 •

edited

Loading