-
-
Notifications
You must be signed in to change notification settings - Fork 246
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[14.0][IMP] add delivery_nfe volumes #3091
Conversation
Hi @mbcosta, |
93bb591
to
4527e1f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ta tranquilo vc prototipar ou ate atender clientes com strings dos campos em português. Porem hoje, na OCA, a gente ja tem que pensar em empresas multi-pais usando e nisso é bom uar strings em ingles (apenas se for diferente do nome da variável) e traduzir por cima. Não avaliei a questão da lógica das coisas que vc ta propondo aqui.
aade86a
to
7d2b6d4
Compare
resolvido! |
@mbcosta acho que consegui corrigir todos os pontos que você levantou, obrigado pelo review! |
@DiegoParadeda pode fazer um rebase e squash? |
ai tb pode se adicionar nos contribuidores do módulo porque é uma contribuição significativa (apesar disso eu não revisei direito ainda). |
ea1c8e3
to
3c43aa7
Compare
na real, o modulo era muito pequeno e muito simples e vcs fizeram até mais codigo do que a gente nele... Nisso eu diria que pode até adicionar a KMEE de co-autor. Novamente não revisei, isso é assumindo que ta legal... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Já que estamos colocando dependências, poderia incluir o product_brand? https://github.com/OCA/brand/tree/16.0/product_brand
- Não faz sentido ter os campos em ambos os modelos dos produtos, template e variante. Coloque somente onde esta o peso (se lembro de cabeça fica na variante);
@DiegoParadeda melhor colocar os campos na variante, como https://github.com/odoo/odoo/blob/14.0/addons/product/models/product.py#L118 A marca se formos usar a dependência, via ficar no template, mas isso pode ser alterado por um terceiro módulo. |
eu diria que pode depender do product_brand. Novamente é um modulo muito utilizado e trivial. Sendo que o proprio l10n_br_delivery_nfe é um modulo muito pequeno tb. |
Concordo também com esta dependencia. |
@DiegoParadeda pode incluir nas dependências a integração com o módulo: https://github.com/OCA/delivery-carrier/tree/14.0/delivery_package_number Da forma como esta modelado a informação do nº de volumes no picking fica inexistente, vejo duas opções: 1 - Criar um campo calculado que leia as informações totais do nº de volumes do documento fiscal; |
Entretanto esse módulo coloca um conjunto de wizards: https://github.com/OCA/delivery-carrier/tree/14.0/delivery_package_number/wizard Em que a integração com ele pode não ficar boa e seja exigido um modelo concreto relacionado ao picking. |
132441b
to
c3d28c5
Compare
ac68417
to
f0d9208
Compare
f0d9208
to
dd4867f
Compare
This PR has the |
dd4867f
to
10f9b01
Compare
considerando os fixes recentes e as aprovações... /ocabot merge major |
On my way to merge this fine PR! |
Congratulations, your PR was merged at 303369f. Thanks a lot for contributing to OCA. ❤️ |
Adiciona volumes (nfe40_vol) gerados automaticamente via: