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

[14.0][IMP] l10n_br_account_payment_order: cnab_codes Santander [240 e 400] #2871

Merged

Conversation

kaynnan
Copy link
Contributor

@kaynnan kaynnan commented Jan 17, 2024

@marcelsavegnago @mbcosta @rvalyi @antoniospneto @douglascstd

PR contém ligação com a revisão do @mbcosta na PR #2848, onde foi realizado a refatoração na PR #2870, logo não estava presente o cnab_codes do Santander, realizei a implementação seguindo o material:

Layout Cobrança - Santander CNAB 240
Layout Cobrança - Santander CNAB 400

@OCA-git-bot
Copy link
Contributor

Hi @mbcosta,
some modules you are maintaining are being modified, check this out!

@kaynnan kaynnan marked this pull request as ready for review January 17, 2024 00:30
@kaynnan kaynnan force-pushed the 14.0-imp-l10n_br_account_payment_order branch from 1e10916 to 2705d6c Compare January 17, 2024 04:18
@kaynnan kaynnan changed the title [14.0][IMP] l10n_br_account_payment_order: cnab_codes Santander 400 [14.0][IMP] l10n_br_account_payment_order: cnab_codes Santander [240 e 400] Jan 17, 2024
Copy link
Contributor

@antoniospneto antoniospneto left a comment

Choose a reason for hiding this comment

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

LGTM.

Obs: pra quem já tem os dados inseridos, pode haver necessidade de ação manual,
pois na hora de atualizar vai dar erro de instrução duplicada.

@mbcosta
Copy link
Contributor

mbcosta commented Jan 17, 2024

Valeu @kaynnan obrigado pelo PR e considerar a revisão feita, tem uma questão simples o seu PR está alterando o l10n_br_account_payment_order/README.rst e l10n_br_account_payment_order/static/description/index.html o que é desnecessário, isso deve ter ocorrido porque o commit foi feito em uma pasta desatualizada para resolver você pode baixar novamente o repositório e copiar esses arquivos( talvez um git pull --rebase origin 14.0 pode resolver ):

$ git clone -b 14.0 https://github.com/OCA/l10n-brazil.git Nome_da_nova_pasta
$ cd pasta_desse_PR
$ cp [l10n_br_account_payment_order/manifest.py Nome_da_nova_pasta/l10n_br_account_payment_order/
$ cp l10n_br_account_payment_order/data/cnab_codes/banco_santander_cnab_240_400.xml Nome_da_nova_pasta/l10n_br_account_payment_order/data/cnab_codes/
$ cd Nome_da_nova_pasta
$ git checkout -b 14.0-imp-l10n_br_account_payment_order
Antes do commit você pode verificar ser a alteração está afetando apenas o manifest e a inclusão do arquivo usando
$ git status ( aqui não deve mostrar o README e o index )
$ git diff
Depois pode ser feito o commit e faz um force push aqui
$ git commit -m "Blablabla"
$ git push escodoo 14.0-imp-l10n_br_account_payment_order:14.0-imp-l10n_br_account_payment_order -f

@kaynnan kaynnan force-pushed the 14.0-imp-l10n_br_account_payment_order branch from 2705d6c to c1e12ae Compare January 17, 2024 19:42
@kaynnan
Copy link
Contributor Author

kaynnan commented Jan 17, 2024

Obrigado pela revisão pessoal.

@mbcosta segui suas orientações, rodei o comando git pull --rebase origin/14.0 e agora encontra-se da maneira correta, envolvendo somente o manifest e o novo arquivo do cnab_codes.

Copy link
Contributor

@mbcosta mbcosta left a comment

Choose a reason for hiding this comment

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

Valeu @kaynnan , obrigado por colocar a Fonte dos dados no arquivo isso é fundamental para um consulta ou atualização

@rvalyi
Copy link
Member

rvalyi commented Jan 17, 2024

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 14.0-ocabot-merge-pr-2871-by-rvalyi-bump-minor, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 28d034e into OCA:14.0 Jan 17, 2024
7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at d56eb2f. 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