Skip to content
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

[12.0][ADD] l10n_br_nfe_spec clean initial addition #981

Merged
merged 10 commits into from
Jan 29, 2021

Conversation

rvalyi
Copy link
Member

@rvalyi rvalyi commented Oct 11, 2020

l10n_br_nfe_spec limpo para ser usado no l10n_br_nfe e modulos de SPED eventualmente. Testes e melhor README vindo na tarde...

@rvalyi rvalyi changed the title [ADD] l10n_br_nfe_spec clean initial addition [12.0][ADD] l10n_br_nfe_spec clean initial addition Oct 11, 2020
@mileo
Copy link
Member

mileo commented Nov 4, 2020

@rvalyi vc esta começando o PR de novo é isso? Nada do que o @gabrielcardoso21 e eu fizemos no #790 vc vai aproveitar?

@mileo
Copy link
Member

mileo commented Nov 4, 2020

@rvalyi vc esta começando o PR de novo é isso? Nada do que o @gabrielcardoso21 e eu fizemos no #790 vc vai aproveitar?

Depois da uma olhada no histórico: https://github.com/odoo-brazil/l10n-brazil/commits/feature/l10n_br_nfe_spec/l10n_br_nfe_spec

@rvalyi
Copy link
Member Author

rvalyi commented Nov 4, 2020

Na vdd se eu nao me engano, no spec nao tinha sido feito nada relevante nao. Tinha que xsd=True de vcs mas que nao discrimina nenhuma informaçao como eu tinha comentado no Telegram.

O @gabrielcardoso21 fez varias melhorias super bem vindas no spec_driven_model e l10n_br_nfe e a a maioria delas vamos re-aproveitar sim. So que tinha sido proposto por voces em outros PRs/branches tb. So que conforme e tinha comentado o problema dessas outras branchs como a https://github.com/odoo-brazil/l10n-brazil/tree/12.0-l10n_br_nfe é que vcs meteram o louco com funcionalidades novas em vez de trabalhar em acertar as coisas para rolar merge primeiro. E so vc e as outras pessoas com quem vc trabalha procurar ver um pouco nos outros repos da OCA para ver que isso nao é uma forma legal de trabalhar, sobre-carrega os PSCs do projeto para revisar e filtrar as coisas e no final das contas atrasa tudo. Comentei que teria sido muito melhor vcs terem feito uma nova branch com as coisas de DFe por examplo em vez de continuar enfiando coisas no l10n_br_nfe e no erpbrasil.edoc sem procurar primeiro botar nem que seja uma CI com 30% de test coverage, nem procurar usar uma versao decente do generateDS ou botar as coisas verde no PR primeiro.

Na Akretion a gente estabilizou uma branch disso antes de vcs partirem para a DFe e outras coisas que é esse:
https://github.com/akretion/l10n-brazil/tree/12.0-l10n_br_nfe-cherrypick-hacks

Infelizmente teremos um certo trabalho depois para juntar as coisas. Mas novamente vamos dar a prioridade na branch que focou em resolver as coisas em vez de acumular funcionalidades novas. Mesmo que tem uma coisa ruim na nossa branch que foi a renomeaçao do modulo l10n_br_nfe_spec para l10n_br_spec_nfe que depois a genteviu que nao era uma boa. Mas dando o rebase com esse PR, esse problema vai morrer nas duas branches.

O lance do l10n_br_nfe_spec é que se trata de um modulo na verdade trivial que deve ser facil de revisar antes da gente resolver esses outros assuntos pendentes. O que eu fiz foi de conseguir fazer como que os modulos spec nao dependesse mais do modulo spec_driven_model que traz muito mais opinioes e pode demorar mais para rolar o merge o que vc pode nao querer usar sempre. Sei la pro sped alguem pode querer usar as constantes do l10n_br_nfe_spec sem o modulo spec_driven_model por exemplo.

Enfim prometo retomar nisso logo. So acho uma pena que @mileo uns meses atras em vez de dar um time para resolver essas coisas vc fazia pressao para fazer merge super irresponsavel para as taxas dedutiveis (compara os conceitos e a quantidade de codigo que vc queria enfiar no l10n_br_fiscal que ja é o maior modulo da OCA com o que Renato fez com talvez 20 linhas usando as posiçoes fiscais) ou outras coisas quase que do mesmo nivel, coisas que nenhum outro repo ia aceitar. E depois quando a gente nao toma tempo para imediatamente desarmar as "pautas bombas" vc acaba fazendo merges em nosso modulos de qualquer jeito como as coisas de evento ou DFe no l10n_br_fiscal que da muito mais trabalho pro @renatonlima limpar uma vez que tem gente usando em produçao.

Falando bem serio ta bem complicado isso @mileo . Vc sempre quer começar 1000 coisas mas raramente bota as coisas numa qualidade decente quando as coisas sao complexas como a parte fiscal, NFe ou CNAB ou RH ou SPED ou sei la o que e ai vc ja parte para outra coisa como a migraçao agora e no fim da conta é sempre a Akretion que tem que ficar correndo atras para botar as coisas numa qualidade digna de ser chamado de OCA. Isso acaba nos sobre-carregendo. Como seria o projeto se a gente nao fizesse esse trabalho todo, seria igual os modulos do fork KMEE na v10 sem teste nem modularidade que vcs largaram por ai?

Agora ja que apesar da conversa que vc e o @renatonlima tiveram sobre a migraçao vc decidiu meter o louco com a migraçao, infelizmente tou dando um gas para limpar os modulos simples que poderiam ser migrados primeiros para nao criar um pesadelo de manutençao daqui 1 ou 2 anos que ainda tera pessoas na v12 com certeza...

@rvalyi
Copy link
Member Author

rvalyi commented Nov 4, 2020

@mileo sei que é um pouco duro, mas para vc realmente ver que nao tou inventando, lembra como os testes falahando na CI do modulo de vcs l10n_br_portal atrasou a vida de todo mundo e principalmente a nossa que fazia a migraçao dos modulos? Bem agora uns 10 dias atras vc sozinho vez o merge do l10n_br_website_sale, veja como ficou legal monitorar os warnings no projeto agora: https://travis-ci.com/github/OCA/l10n-brazil/jobs/427240936#L497

(No caso dos warning do l10n_br_fiscal com os arquivos nao carregado pelo manifest, a gente sabe do problema, mas é um assunto porem mais complexo que talvez vai precisar de um PR no pylint-odoo (sendo que sem essa mudança do Clemente os testes ia levar 10 minutos a mais). Enfim nao é apenas uma falta de atençao.)

@mileo
Copy link
Member

mileo commented Nov 4, 2020

Na vdd se eu nao me engano, no spec nao tinha sido feito nada relevante nao. Tinha que xsd=True de vcs mas que nao discrimina nenhuma informaçao como eu tinha comentado no Telegram.

O @gabrielcardoso21 fez varias melhorias super bem vindas no spec_driven_model e l10n_br_nfe e a a maioria delas vamos re-aproveitar sim. So que tinha sido proposto por voces em outros PRs/branches tb. So que conforme e tinha comentado o problema dessas outras branchs como a https://github.com/odoo-brazil/l10n-brazil/tree/12.0-l10n_br_nfe é que vcs meteram o louco com funcionalidades novas em vez de trabalhar em acertar as coisas para rolar merge primeiro. E so vc e as outras pessoas com quem vc trabalha procurar ver um pouco nos outros repos da OCA para ver que isso nao é uma forma legal de trabalhar, sobre-carrega os PSCs do projeto para revisar e filtrar as coisas e no final das contas atrasa tudo. Comentei que teria sido muito melhor vcs terem feito uma nova branch com as coisas de DFe por examplo em vez de continuar enfiando coisas no l10n_br_nfe e no erpbrasil.edoc sem procurar primeiro botar nem que seja uma CI com 30% de test coverage, nem procurar usar uma versao decente do generateDS ou botar as coisas verde no PR primeiro.

Na Akretion a gente estabilizou uma branch disso antes de vcs partirem para a DFe e outras coisas que é esse:
https://github.com/akretion/l10n-brazil/tree/12.0-l10n_br_nfe-cherrypick-hacks

Infelizmente teremos um certo trabalho depois para juntar as coisas. Mas novamente vamos dar a prioridade na branch que focou em resolver as coisas em vez de acumular funcionalidades novas. Mesmo que tem uma coisa ruim na nossa branch que foi a renomeaçao do modulo l10n_br_nfe_spec para l10n_br_spec_nfe que depois a genteviu que nao era uma boa. Mas dando o rebase com esse PR, esse problema vai morrer nas duas branches.

O lance do l10n_br_nfe_spec é que se trata de um modulo na verdade trivial que deve ser facil de revisar antes da gente resolver esses outros assuntos pendentes. O que eu fiz foi de conseguir fazer como que os modulos spec nao dependesse mais do modulo spec_driven_model que traz muito mais opinioes e pode demorar mais para rolar o merge o que vc pode nao querer usar sempre. Sei la pro sped alguem pode querer usar as constantes do l10n_br_nfe_spec sem o modulo spec_driven_model por exemplo.

Enfim prometo retomar nisso logo. So acho uma pena que @mileo uns meses atras em vez de dar um time para resolver essas coisas vc fazia pressao para fazer merge super irresponsavel para as taxas dedutiveis (compara os conceitos e a quantidade de codigo que vc queria enfiar no l10n_br_fiscal que ja é o maior modulo da OCA com o que Renato fez com talvez 20 linhas usando as posiçoes fiscais) ou outras coisas quase que do mesmo nivel, coisas que nenhum outro repo ia aceitar. E depois quando a gente nao toma tempo para imediatamente desarmar as "pautas bombas" vc acaba fazendo merges em nosso modulos de qualquer jeito como as coisas de evento ou DFe no l10n_br_fiscal que da muito mais trabalho pro @renatonlima limpar uma vez que tem gente usando em produçao.

Falando bem serio ta bem complicado isso @mileo . Vc sempre quer começar 1000 coisas mas raramente bota as coisas numa qualidade decente quando as coisas sao complexas como a parte fiscal ou CNAB ou RH ou SPED ou sei la o que e ai vc ja parte para outra coisa como a migraçao agora e no fim da conta é sempre a Akretion que tem que ficar correndo atras para botar as coisas numa qualidade digna de ser chamado de OCA. Isso acaba nos sobre-carregendo. Como seria o projeto se a gente nao fizesse esse trabalho todo, seria igual os modulos do fork KMEE na v10 sem teste nem modularidade que vcs largaram por ai?

Agora ja que apesar da conversa que vc e o @renatonlima tiveram sobre a migraçao vc decidiu meter o louco com a migraçao, infelizmente tou dando um gas para limpar os modulos simples que poderiam ser migrados primeiros para nao criar um pesadelo de manutençao daqui 1 ou 2 anos que ainda tera pessoas na v12 com certeza...

Bom veja a arvore de commits, o xsd=True foi melhor forma computacionalmente de descorbrir que o campo é um campo do XSD e precisa ser exportado.

Alem disso houveram outras melhorias, mas pude ver que várias delas vocês precisaram utilizar na branch de vocês: https://github.com/akretion/l10n-brazil/commits/12.0-l10n_br_nfe-cherrypick-hacks

Nos trabalhamos até onde foi possível no PR da NF-e e como você todo dia comenta que esta trabalhando nele fomos para os próximos passos. Estamos mais ou menos desde abril com isso parado, mas se você precisar de ajuda estamos a disposição.

Esses próximos passos que trabalhamos geraram várias melhorias importantes:

Sei que vocês tem o ritmo de vocês, de gerar receita através de projetos fora do pais e bancar o P&D da localização.

Nosso foco de trabalho é 100% Brasileiro e nossos clientes precisam desses avanços assim como nosso fluxo de caixa. Nossa intenção é fazer o melhor para eles, nossos funcionários e os usuários da localização.

Concordo que o PR do MD-E foi um colateral indesejado, mas ele começou em 2 de Março de 2020 e for feito o merge 28 Sep de 2020. Será que ninguém do time de vocês tinha 5 minutos para comentar sobre isso ou sobre a questão dos eventos?

Sobre a migração eu comentei com o @renatonlima que temos um projeto em andamento que tem interesse na v14. Alem disso você pode verificar a última mensagem que eu te mandei no Hangouts:

image

A ideia de começar a migração para v14 é para anteciparmos os problemas, implementar coisas na v12 que podem facilitar a migração da v14 (Como o PR #1010 que precisa de correções). Se a gente começar a migração pra 14 ano que vem, alguns desses problemas só vamos ficar sabendo e resolvendo no futuro.

@rvalyi
Copy link
Member Author

rvalyi commented Nov 4, 2020

vc fala de ritmo Luis, mas da uma olha nos commits do projeto para ver quem sustenta esse "ritmo" ai faz 10 anos: https://github.com/OCA/l10n-brazil/graphs/contributors
(o mbcosta é o Magno da Akretion tb para quem ler e nao sabe e tem muitos PRs quase OK para merge tb ainda).
ve aqui quem bolou uma estrura de empresa para tornar a OCA viavel https://odoo-community.org/shop/page/20?search=akretion&version=14

Perfeito vcs terem feitos os trabalhos com cielo, a DRE ou de website, fora os warning ninguem reclama disso. A questao nao é essa. A questao é quando vcs pegam problemas mais complexos e que nao da conta de fazer correctamente e acabam atrasando a vida de geral.

Nao é verdade que vc pode pegar um projeto como vcs fizeram na v10 de voces, jogar na OCA e que por algum milagro isso vai acabar se auto limpando ara virar algo limpo. Depois que esta em produçao com o modelo de dados zoado, sem teste e sem modularidade, o trabalho de um vai quebrar tudo do trabalho do outro nao tem como isso dar certo. Eu acho que temos bem mais experiença professional ou com open source para saber disso. Sendo que nem todos projetos tem centenas de milares de R$ de budget para seguir qualquer refator maluco ou custo de manutençao proibitivo uma vez os extrapolos feitos na prod.

Sobre as taxas dedutiveis, temos projetos da França que fazem isso desde a v5 (projeto feitos aqui no Brasil), e na v8 chegamos a fazer uns patches para fazer isso aqui tb. Eu acho que quem nao sabia que o Odoo fazia isso desde a v5 e foi re-inventar a roda contornando completamente o modulo account é voce Luis. Se a gente demorou para olhar os seu PR é porque ele era um monstro que nehnum projeto da OCA nunca teria aceitado e dificilmente teria olhado tb.

O que vc nao pode fazer Luis é fazer o projeto de refem com seus equivocos comerciais. Em 2020 80% da P&D da localizaçao que a gente faz é bancado por projetos do Brasil mesmo (ja foi diferente atras sim). Vcs ja se equivocaram de pegar um projeto enorme na v10 que vcs so conseguiram atender re-escrevendo tudo do Odoo e largando a OCA e que a gente teve a sabedoria de recusar. Por muito pouco e muitos esforços nossos nao foi OCA/l10n-brazil que nao pagou o pato desse equivoco, o que com certeza teria feito um fork ganhar da OCA como resultado.

Agora ameaçam de duplicar a base de codigo de um dos 3 maiores projetos da OCA com a migraçao anticipada da 14. Quando vc mandou sua mensagem de forma privada, o Renato tava levando o pai dele no hospital, na mesma semana das apresentaçoes da OCA, ninguem te deve resposta em canal privado em tempo e hora. Depois vcs conversaram e ainda assim vc largou as coisas da v12 para fazer o que quer.

Queremos migrar para a v14 e quanto antes tb. Nao achamos nada demais pedir uns dias ainda antes de aceitar os PRs da 14. O que a gente quer apenas é que esses PRs da 14 sejam re-feitos daqui uns dias para manter as duas bases de codigo o mais proximo possivel. Principalemente pros modulos l10n_br_fiscal e l10n_br_nfe que ainda vao mudar um pouquinho na v12. O modelo de dados provavelemente nao vai mudar entao vcs devem conseguir adaptar na prod nessas primeiras se semanas. O que nao da para aceitar é vc fazer nem 20% do trabalho desses modulos e exigir de duplicar o nosso trabalho so para atender seus caprichos comerciais. Gambiarras para atender clientes todos fazemos. O que nao da é jogar isso na OCA como se vc o projeto do seus clientes e que se danem os outros.

@@ -0,0 +1 @@
from . import models
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
from . import models
from . import models

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

valeu Marcel, porem nem tinha botado para revisar ainda. Principalmente vou subir os testes de importaçao. Foi onde eu trabalhei para que os testes possam rodar sem o spec_driven_model. Mas essa semana acertamos isso com certeza.

Copy link
Member

@marcelsavegnago marcelsavegnago Nov 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rvalyi Opaa.. fico feliz com a noticia :).

Vlw

@rvalyi rvalyi force-pushed the 12.0-add-l10n_br_nfe_spec branch 6 times, most recently from 5219a1d to 7492ac8 Compare December 7, 2020 16:15
@rvalyi rvalyi mentioned this pull request Dec 9, 2020
@rvalyi
Copy link
Member Author

rvalyi commented Dec 10, 2020

valeu @marcelsavegnago vou tb comentar algo sobre os testes e referenciar o modulo l10n_br_nfe

@mileo
Copy link
Member

mileo commented Dec 15, 2020

@rvalyi pode fazer um rebase por gentileza?

@renatonlima renatonlima force-pushed the 12.0-add-l10n_br_nfe_spec branch 2 times, most recently from 03f0fd1 to b10c6e2 Compare January 26, 2021 20:30
@rvalyi
Copy link
Member Author

rvalyi commented Jan 27, 2021

opa, o ultimo fix que eu comentei no comentario #981 (comment) tinha sido comido por um dos ultimos rebases. Eu acabei de refazer ele e tb completar o README. Agora ta verde. Estamos testando mais ele do nosso lado com o PR do l10n_br_nfe. Tendo tests bem OK com o l10n_br_nfe seria bom fazer o merge desse...

@marcelsavegnago
Copy link
Member

marcelsavegnago commented Jan 27, 2021

@rvalyi Esse módulo está rodando bem legal.. no começo dos meus testes até cogitei alguma gambiarra para contornar algo mas nem precisei 😄 . No máximo, acabei incluindo a classe ICMS00 para poder efetuar teste completo da geração da NFe mas acredito que com a mudança comentada por você isto será tratado pelo módulo l10n_br_nfe.

Acho que está ok para revisão e merge. @mileo, se teu time puder priorizar esta PR acho que já vamos dar uma avançada boa em relação a NFe.

@rvalyi
Copy link
Member Author

rvalyi commented Jan 27, 2021

entao @marcelsavegnago tem mais uma pegadinha, esse modulo usa o nfelib 100% gerido https://github.com/akretion/nfelib e nao o fork da Kmee/erpbrasil que tem gambiarras manuais e usou um generateDS super desatualizado e com patches.

o https://github.com/akretion/nfelib so funciona com o erpbrasil.edoc se usar esse PR: erpbrasil/erpbrasil.edoc#30 que ainda nao foi revisado pela KMEE...

Entao por mim beleza, mas tem que ver se isso nao pega com quem nao usar esses nfelib e erpbrasil.edoc.

@marcelsavegnago
Copy link
Member

entao @marcelsavegnago tem mais uma pegadinha, esse modulo usa o nfelib 100% gerido https://github.com/akretion/nfelib e nao o fork da Kmee/erpbrasil que tem gambiarras manuais e usou um generateDS super desatualizado e com patches.

o https://github.com/akretion/nfelib so funciona com o erpbrasil.edoc se usar esse PR: erpbrasil/erpbrasil.edoc#30 que ainda nao foi revisado pela KMEE...

No meu ambiente estou utilizando o nfelib mantido pela akretion e já estou usando tbm a PR30 do erpbrasil.edoc.

Então por mim beleza, mas tem que ver se isso não pega com quem não usar esses nfelib e erpbrasil.edoc.

Concordo.

@rvalyi
Copy link
Member Author

rvalyi commented Jan 27, 2021

entao @marcelsavegnago tem mais uma pegadinha, esse modulo usa o nfelib 100% gerido https://github.com/akretion/nfelib e nao o fork da Kmee/erpbrasil que tem gambiarras manuais e usou um generateDS super desatualizado e com patches.
o https://github.com/akretion/nfelib so funciona com o erpbrasil.edoc se usar esse PR: erpbrasil/erpbrasil.edoc#30 que ainda nao foi revisado pela KMEE...

No meu ambiente estou utilizando o nfelib mantido pela akretion e já estou usando tbm a PR30 do erpbrasil.edoc.

Então por mim beleza, mas tem que ver se isso não pega com quem não usar esses nfelib e erpbrasil.edoc.

Concordo.

Nesse PR eu botei o nfelib da akrteion/nfelib pros testes passarem, porem nao alterei o erpbrasil.edoc. Caso nao houver merge, talvez seja necessario antes de rolar esse merge para deixar as coisas explicitas.

@rvalyi rvalyi added the blocked label Jan 27, 2021
@rvalyi
Copy link
Member Author

rvalyi commented Jan 27, 2021

@marcelsavegnago na vdd depois de 6 meses emitindo notas em produção descobrimos 2 bugs importantes nesse modulo bem hoje. Daqui pouco eu falo mais sobre. Deve ser fácil resolver.

@rvalyi rvalyi removed the blocked label Jan 28, 2021
@rvalyi
Copy link
Member Author

rvalyi commented Jan 28, 2021

pessoal, nao tem bug nehnum nesse modulo nao. Eh que ontem a gente nao tava conseguindo prencher uma nota de exportaçao porque o popup da informaçao de exportaçao tava vazio. Mas isso era so porque o codigo que o @gabrielcardoso21 adiciondou no spec_driven_model para acrescentar o botao export_xml no spec_view.py criou uma regressao nessas telas com um sheet como ponto de ancoragem. Vou fazer um outro PR para corrigir isso no spec_model_driven e remover o label blocker. Por mim o modulo ta bem OK entao!

cc @marcelsavegnago @renatonlima

@marcelsavegnago
Copy link
Member

ping @mileo @gabrielcardoso21

@@ -0,0 +1,14 @@

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add header?


# Código de Regime Tributário.
# Este campo será obrigatoriamente preenchido com:
CRT_EMIT = [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An improvement that could be made in the future is split the constants code and class objects

@@ -10,5 +10,5 @@ nfselib.ginfes==0.2.0
nfselib.issnet==0.2.0
xmldiff==2.4
lxml==4.6.2
-e git://github.com/erpbrasil/nfelib.git#egg=nfelib
-e git://github.com/akretion/nfelib.git@master_gen_v4_00#egg=nfelib
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be important to leave the official lib or switch to akretion/nfelib (and merge it into the erpbrasil repo) because it can make installation difficult

Copy link
Member Author

@rvalyi rvalyi Jan 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

problem is KMEE is using the erpbrasil/nfelib fork in production, if we force the merge in their erpbrasil.edoc repo we may break their stuff in production. They didn't revised erpbrasil/erpbrasil.edoc#30

I'm a bit pissed off to have spent time showing generateDS and akretion/nfelib to them and now get the project tied to a sub optimal nfelib forked version full of manual hacks again while the whole purpose of the code generation technology is to get rid of manual coding and manutention. But we can give them more time to make the switch.

Also I criticized several times that erpbrasil.edoc has hard dependencies with the binding libs. So basically we started depending on erpbrasil.edoc for some small nfse modules while the nfe part was still not clean and now we have exactly the situation I was warning about...

At the same time, it seems KMEE is using its own 12 staging branch instead the OCA 12 branch, so I would say we could still merge this one while leaving them more tome to make the switch in the erpbrasil.edoc repo.

Copy link
Member Author

@rvalyi rvalyi Jan 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one more consideration in favor of the merge: in fact, at this stage using akretion/nfelib will only affect if the tests of this module pass or not (tests will only work with akretion/nfelib) but if KMEE wants to use erpbrasil/nfelib in their own requirements.txt, the code of the module will still be working exactly the same, because in fact it is quite independant from its bindings lib. So I would suggest a quick merge as this will not even impact people still using erpbrasil/nfelib in their production combo. cc @renatonlima @marcelsavegnago .

Note: the tests in this module are similar to the ones in nfelib: I read some NFe's XML and convert them to Python objects, But this time I convert them to an abstract mixin structure that reflects naively the XML structure because the mixins are note injected in concrete Odoo models, as this only happens in l10n_br_nfe. So the import system is fairly simple enough and implemented in the test itself.

@renatonlima
Copy link
Member

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 12.0-ocabot-merge-pr-981-by-renatonlima-bump-nobump, awaiting test results.

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@OCA-git-bot OCA-git-bot merged commit 5a0a8d8 into OCA:12.0 Jan 29, 2021
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 7261271. Thanks a lot for contributing to OCA. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants