banking-addons-team team mailing list archive
-
banking-addons-team team
-
Mailing list archive
-
Message #01741
Re: [Merge] lp:~akretion-team/banking-addons/70-banking-addons-fr-lcr into lp:banking-addons
Review: Approve
I found your code perfectly fine to look at, but found some minor things that could be fixed even though not impacting functionality:
#80 'this module' (it's just one)
#141 why not payment_order_id?
#253 French or is this a fixed idiom that's not going to be translated anyways?
#820 str.ljust
#821 'the lenght'
#832, #843 everything that's not an acronym should be lower case
#943, #974 why not just concatenations?
And the reason I could read through your code is that I'm a friend of Freanch literature and happen to understand the variable names you chose. It makes it excruciatingly hard for people not speaking the language. I approve, but please, use Esperanto wich is called English nowadays.
--
https://code.launchpad.net/~akretion-team/banking-addons/70-banking-addons-fr-lcr/+merge/210701
Your team Banking Addons Core Editors is subscribed to branch lp:banking-addons.
References