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

[16.0][MIG] l10n_br_base #2164

Merged
merged 37 commits into from
Oct 24, 2022
Merged

[16.0][MIG] l10n_br_base #2164

merged 37 commits into from
Oct 24, 2022

Conversation

mileo
Copy link
Member

@mileo mileo commented Oct 11, 2022

rvalyi
rvalyi previously requested changes Oct 11, 2022
Copy link
Member

@rvalyi rvalyi left a comment

Choose a reason for hiding this comment

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

@mileo prós 8-10 módulos mais básicos do repo, (como esse) seria ideal fazer que nem na v13, fazer uma etapa intermediária na v15. Quase que não custa nada de fazer a migração em 2 passos mas manda um sinal bem melhor para quem se ferrou com a v15 (por exemplo através de uma implementação feita por um parceiro oficial; não é pouca gente que tá se ferrando) e que assim poderia enxergar alguma saída pela OCA (já que não poderia migrar para a v16 já). Do nosso lado a gente não pretende manter nada na v15, mas TB não iremos blocar caso alguém cuida de fazer os ports e backports na v15. E mesmo se a v15 tiver apenas 10 ou algum dia talvez até 20 módulos já seria alguma para essa galera que foi se perder na v15 sem procurar sobre o projeto antes.

Alem disso seria bom segurar um pouquinho a onda da 16.0 para limpar ainda alguns detalhes na 14.0 (inclusive que vem de vcs). Pois a partir do momento que a gente começa a fazer os merges na 16.0 para valer, considerando os nossos recursos ficaria inviavel fazer backports e frontports com a 12.0 mais. Manter 2 versões ja é o máximo que a gente consegue fazer e não seria bem vc que poderia demostrar o contrario (estão mais de um ano sem revisar quase nada nem resolver nenhum bug no projeto). Então algumas semanas ou meses ainda antes de desligar a 14.0 da 12.0 não faria mal. Se vc ainda considerar que vai levar alguns meses para ter o modulo account_reconciliation_widget na 16.0 tb não adiantaria muito ter os módulos de localização do account tão cedo (o base poderia ate ser um pouco antes). Quem tiver com fogo no rabo pode ajudar na migração do account_reconciliation_widget eu acho que seria muito bem vindo.

cc @renatonlima @marcelsavegnago @netosjb @felipemotter @mbcosta

@mileo mileo force-pushed the 16.0-mig-l10n_br_base branch from c7750de to c370e26 Compare October 19, 2022 01:45
@mileo mileo marked this pull request as ready for review October 20, 2022 19:58
@mileo mileo force-pushed the 16.0-mig-l10n_br_base branch from c370e26 to 387f620 Compare October 20, 2022 20:08
@mileo
Copy link
Member Author

mileo commented Oct 20, 2022

@mileo prós 8-10 módulos mais básicos do repo, (como esse) seria ideal fazer que nem na v13, fazer uma etapa intermediária na v15. Quase que não custa nada de fazer a migração em 2 passos mas manda um sinal bem melhor para quem se ferrou com a v15 (por examplo através de uma implementação feita por um parceiro oficial; não é pouca gente que tá se ferrando) e que assim poderia enxergar alguma saída pela OCA (já que não poderia migrar para a v16 já). Do nosso lado a gente não pretende manter nada na v15, mas TB não iremos blocar caso alguém cuida de fazer os ports e backports na v15. E mesmo se a v15 tiver apenas 10 ou algum dia talvez até 20 módulos já seria alguma para essa galera que foi se perder na v15 sem procurar sobre o projeto antes.

Alem disso seria bom segurar um pouquinho a onda da 16.0 para limpar ainda alguns detalhes na 14.0 (inclusive que vem de vcs). Pois a partir do momento que a gente começa a fazer os merges na 16.0 para valer, considerando os nossos recursos ficaria inviavel fazer backports e frontports com a 12.0 mais. Manter 2 versões ja é o máximo que a gente consegue fazer e não seria bem vc que poderia demostrar o contrario (estão mais de um ano sem revisar quase nada nem resolver nenhum bug no projeto). Então algumas semanas ou meses ainda antes de desligar a 14.0 da 12.0 não faria mal. Se vc ainda considerar que vai levar alguns meses para ter o modulo account_reconciliation_widget na 16.0 tb não adiantaria muito ter os módulos de localização do account tão cedo (o base poderia ate ser um pouco antes). Quem tiver com fogo no rabo pode ajudar na migração do account_reconciliation_widget eu acho que seria muito bem vindo.

cc @renatonlima @marcelsavegnago @netosjb @felipemotter @mbcosta

@rvalyi não iremos nesse momento investir em uma versão intermediária, como a 15.0, a não ser que tenha algum cliente que invista nisso.

Nosso foco nesses próximos dias será revisar os PRs da v14 e migrar os módulos básicos da localização para a v16.

@mileo mileo dismissed rvalyi’s stale review October 20, 2022 20:12

Somente um comentário, sem solicitação de mudança

This was referenced Oct 20, 2022
@mileo mileo force-pushed the 16.0-mig-l10n_br_base branch from 387f620 to 516e3a4 Compare October 23, 2022 21:01
@mileo mileo requested a review from douglascstd October 23, 2022 21:06
@mileo
Copy link
Member Author

mileo commented Oct 23, 2022

/ocabot migration l10n_br_base

@OCA-git-bot OCA-git-bot added this to the 16.0 milestone Oct 23, 2022
@OCA-git-bot OCA-git-bot mentioned this pull request Oct 23, 2022
58 tasks
rvalyi
rvalyi previously requested changes Oct 23, 2022
@@ -20,6 +20,16 @@ class Partner(models.Model):
_name = "res.partner"
_inherit = [_name, "l10n_br_base.party.mixin"]

def _inverse_street_data(self):
"""update self.street based on street_name, street_number and street_number2"""
for partner in self:
Copy link
Member

@rvalyi rvalyi Oct 23, 2022

Choose a reason for hiding this comment

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

A gente ta se esforçando para que os módulos centrais da localização funcionam com bancos multi-pais, nisso
tem que filtrar apenas os registros pros quais partner.country_id.code.upper() == "BR" e senão chamar super.

Eu tb acho melhor falar no docstring o que a gente faz no override do que fazer o papagaio do docstring original.
Poderia ser algo do tipo:
""'"In Brazil the address format is street_name, street_number (comma instead of space)"""

Copy link
Member Author

Choose a reason for hiding this comment

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

Solved

@mileo mileo force-pushed the 16.0-mig-l10n_br_base branch 2 times, most recently from ed68f45 to 9f690d2 Compare October 24, 2022 00:31
@mileo mileo requested a review from rvalyi October 24, 2022 00:50
Copy link
Member

@douglascstd douglascstd left a comment

Choose a reason for hiding this comment

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

[REPROVADO] Testes Funcionais
Não aparece na view do contact o campo para Instant Payment Method (PIX)
image

APROVADO

Outros testes realizados:

Inserção e validação do CNPJ:
image

Configuração Localicalização Brasil:

image

Inserção e validação da inscrição estadual

image

@mileo
Copy link
Member Author

mileo commented Oct 24, 2022

[REPROVADO] Testes Funcionais Não aparece na view do contact o campo para Instant Payment Method (PIX) image

APROVADO

Outros testes realizados:

Inserção e validação do CNPJ: image

Configuração Localicalização Brasil:

image

Inserção e validação da inscrição estadual

image

@douglascstd obrigado pela revisão, mas esse campo é adicionado somente pelo módulo l10n_br_account:

https://github.com/OCA/l10n-brazil/blob/14.0/l10n_br_account/views/res_partner_view.xml#L21-L28

@mileo mileo requested a review from douglascstd October 24, 2022 16:00
@mileo mileo force-pushed the 16.0-mig-l10n_br_base branch from b0a13f2 to 0c705dc Compare October 24, 2022 22:14
@mileo
Copy link
Member Author

mileo commented Oct 24, 2022

/ocabot merge nobump

Histórico reescrito com PR da v15

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 16.0-ocabot-merge-pr-2164-by-mileo-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 2c4ee8f into OCA:16.0 Oct 24, 2022
@OCA-git-bot
Copy link
Contributor

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

@rvalyi
Copy link
Member

rvalyi commented Nov 28, 2024

IMPORTANTE: o procedimento tecnico para fazer o PR não foi correto e isso perdeu mais de 550 commits. Explicamos e corrigimos o problema aqui

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.

9 participants