openerp-brazil-team team mailing list archive
-
openerp-brazil-team team
-
Mailing list archive
-
Message #01272
Re: [Merge] lp:~fnegrini/openerp.pt-br-localiz/openerp.pt-br-localiz into lp:openerp.pt-br-localiz
Review: Needs Fixing
* text_only_number deve ser renomeada para "only_number" ser uma função e definida como algo:
delete_chars = string.punctuation + ' '
def only_number(s):
return str(s).translate(None, delete_chars)
* text_normalize deve ser uma função tb e pode se chamar normalize.
* Remover os or '', eles são necessários?
* invoice_access_key poderia usar uma só interpolação de string para fazer todas as justificações e fazer strip das strings.
* quem sabe ao invés de centenas de linhas duplas usar uma função:
def subelement_text(root, name, text):
t = SubElement(root,name)
t.text = text
return t
Eu sei que isso seria mais um refactoring, mas faria o código ficar tão mais limpo que valeria a pena.
* usar enumerate no for ao invés de manter um indice manual
* remover linhas em branco desnecessárias, como 414, 469
* Usar a ferramenta pep8 para verificar a formatação do código e a pyflakes para mostrar variaveis desnecessárias
* linha 474: nfeProc.extend([NFe]) para nfeProc.append(NFe)
Isso foi só um review rápido eu devo ter perdido vários detalhes a mais, mas fazendo as revisões propostas acima acho que os outros detalhes vão ficar mais visiveis.
--
https://code.launchpad.net/~fnegrini/openerp.pt-br-localiz/openerp.pt-br-localiz/+merge/80941
Your team OpenERP Brazil Team is subscribed to branch lp:openerp.pt-br-localiz.
References