← Back to team overview

openerp-brazil-team team mailing list archive

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