Feature/l10n fixes

[mbehrle]

Fixes https://github.com/openlabs/nereid/issues/197

[coveralls]

Coverage decreased (-0.32%) when pulling 5a5cf2cf77a2d5d26ca91d3c734c980ddd8c2b2d on mbehrle:feature/l10n_fixes into 3de86b00d39a55d7831a4f2ebae97e345100d75b on openlabs:develop.

[coveralls]

Coverage decreased (-0.33%) when pulling f5fe9e0f287e1b230f9db8e9ac5d5e1cab0defc8 on mbehrle:feature/l10n_fixes into 3de86b00d39a55d7831a4f2ebae97e345100d75b on openlabs:develop.

[coveralls]

Coverage decreased (-0.32%) when pulling 29c0b5acf91e7f5ba86e14d846d6b7d67f6a735a on mbehrle:feature/l10n_fixes into 3de86b00d39a55d7831a4f2ebae97e345100d75b on openlabs:develop.

[sharoonthomas]

This was a long overdue cleanup and I dont understand translations well enough.

The build seems to have failed because of flake8 issues. Could you fix them too and update the pr ? In the meantime reviewing the patch.

[mbehrle]

• Sharoon Thomas: " Re: [nereid] Feature/l10n fixes (#243)" (Sun, 21 Dec 2014 22:07:37 -0800):

This was a long overdue cleanup and I dont understand translations well enough.

After having digged in now for a while, I think the overall concept is a rather good solution.

The build seems to have failed because of flake8 issues. Could you fix them too and update the pr ?

Of course, will do ASAP.

In the meantime reviewing the patch.

Thanks.

Another question: some modules carry a i18n directory with a pt_BR translation. Are they functional? Shouldn't they be removed/migrated to use the locale dir?

Mathias Behrle
PGP/GnuPG key availabable from any keyserver, ID: 0x8405BBF6

[mbehrle]

• Mathias Behrle: " Re: [nereid] Feature/l10n fixes (#243)" (Mon, 22 Dec 2014 12:32:14 +0100):

• Sharoon Thomas: " Re: [nereid] Feature/l10n fixes (#243)" (Sun, 21 Dec 2014 22:07:37 -0800):

This was a long overdue cleanup and I dont understand translations well enough.

After having digged in now for a while, I think the overall concept is a rather good solution.

The build seems to have failed because of flake8 issues. Could you fix them too and update the pr ?

Of course, will do ASAP.

Now had a closer look to those flake8 issues.

Indeed I have set in my .vimrc ignore="E128,E123,E126"

and the reviewbot on tryton.org has set self.options.ignore = ('E122', 'E123', 'E124', 'E126', 'E128')

At least excluding E123, E128 should be really reasonable (even Barry Warsaw recommends it;) [0][1]. Fixing those issues gives as ugly code as in [3]. Flake8 seems to be overzealous itself and even check more than pep8 [2], so "ignore": ["E123", "E226", "E24"] should be reasonable as well.[2]

I would propose to at least ignore E123, E128 in tox.ini, I will fix the other issues in the meantime.

Just let me know how you think about it.

[0] https://github.com/jcrocholl/pep8/issues/103 [1] https://github.com/jcrocholl/pep8/pull/207 [2] https://sublime.wbond.net/packages/Python%20Flake8%20Lint [3]

--- a/trytond_nereid/translation.py +++ b/trytond_nereid/translation.py @@ -79,8 +79,8 @@ class Translation: pool = Pool() ModelData = pool.get('ir.model.data') models_data = ModelData.search([

• ('module', '=', module),
• ])
• ('module', '=', module), ]
• ) fs_id2prop = {} for model_data in models_data: fs_id2prop.setdefault(model_data.model, {}) @@ -100,12 +100,12 @@ class Translation: module_translations = cls.search([ ('lang', '=', lang), ('module', '=', module),
• ], order=[])
• ], order=[]) for translation in module_translations: key = translation.unique_key if not key: raise ValueError('Unknown translation type: %s' %
• translation.type)
• translation.type) key2ids.setdefault(key, []).append(translation.id)

Mathias Behrle
PGP/GnuPG key availabable from any keyserver, ID: 0x8405BBF6

[coveralls]

Coverage decreased (-0.32%) when pulling b790586e14ed2b7eb0ce64517d90c560e2f82d15 on mbehrle:feature/l10n_fixes into 3de86b00d39a55d7831a4f2ebae97e345100d75b on openlabs:develop.

[coveralls]

Coverage decreased (-0.32%) when pulling 837a4881c2b104e400e79a645756087d550849b2 on mbehrle:feature/l10n_fixes into 3de86b00d39a55d7831a4f2ebae97e345100d75b on openlabs:develop.

[sharoonthomas]

Heads up: I will be a little late in reviewing and merging this. Travelling in the next couple of days. Thanks again for your patch and patience.

[mbehrle]

Heads up: I will be a little late in reviewing and merging this. Travelling in the next couple of days. Thanks again for your patch and patience.

Thanks a lot, Sharoon, and a happy Christmas to you and all the nice people at Openlabs!

Mathias Behrle
PGP/GnuPG key availabable from any keyserver, ID: 0x8405BBF6

[sharoonthomas]

Mathis,

Thank you for your greetings and hope you had a good christmas too.

I think the patch can be merged, and thanks for the link to the pep8 discussion on flake8 tool. It was very enlightening and both styles seem to be valid pep8 :) (learn't something new there). From the same discussion everyone seems to be concerned about mixing both styles at the same time. I would really appreciate if we could stick to the current code style just for the sake of consistency. Sorry about the trouble!

Future modules and code bases should use the hanging intends because it is now both valid pep8 and used by the parent project (tryton).

[mbehrle]

• Sharoon Thomas: " Re: [nereid] Feature/l10n fixes (#243)" (Sun, 28 Dec 2014 22:51:11 -0800):

Hi Sharoon,

I think the patch can be merged, and thanks for the link to the pep8 discussion on flake8 tool. It was very enlightening and both styles seem to be valid pep8 :) (learn't something new there). From the same discussion everyone seems to be concerned about mixing both styles at the same time. I would really appreciate if we could stick to the current code style just for the sake of consistency. Sorry about the trouble!

Just to be clear before starting: Do I understand correctly, that you prefer to revert those pep8 changes introduced in https://github.com/mbehrle/nereid/commit/fb36e0c98c9eea065cf27c752be3869a2e407934 ?

They slipped in, because I saw the earlier changes in this respect and I didn't know, if you wanted to keep them... Usually I had indeed just introduced the nereid changes.

Future modules and code bases should use the hanging intends because it is now both valid pep8 and used by the parent project (tryton).

I think this is a good decision. BTW: I am using http://www.b2ck.com/~ced/python.vim, which usually gives good results.

Mathias Behrle
PGP/GnuPG key availabable from any keyserver, ID: 0x8405BBF6