-
-
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][REF] l10n_br_account_payment_brcobranca: Resolvendo erros PEP8 validados em versões recentes #3287
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Hi @rvalyi, |
l10n_br_account_payment_brcobranca/models/account_payment_line.py
Outdated
Show resolved
Hide resolved
l10n_br_account_payment_brcobranca/models/account_payment_order.py
Outdated
Show resolved
Hide resolved
…pass) PEP8 was included logger.Warning but is unnecessary use TRY for this because there no unexpect error, the code already know the Banks that need a specific method.
mbcosta
force-pushed
the
14.0-REF-brcobranca_pep8
branch
from
August 15, 2024 14:28
eeff87b
to
f4b4bea
Compare
…st-timeout) Use of external request method without timeout. It could wait for a long time.'
mbcosta
force-pushed
the
14.0-REF-brcobranca_pep8
branch
from
August 15, 2024 17:31
f4b4bea
to
b63debd
Compare
antoniospneto
approved these changes
Aug 15, 2024
rvalyi
approved these changes
Aug 15, 2024
kaynnan
approved these changes
Aug 16, 2024
/ocabot merge patch |
This PR looks fantastic, let's merge it! |
Congratulations, your PR was merged at 8a4539f. 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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refactoring to solve PEP8 errors in the v16 pre-commit.
Resolvendo erros PEP8 validados em versões recentes do pre-commit v15 v16:
Para resolver esse erro foi incluído o logger.Warning no PR #3225
Mas é desnecessário, acredito que esse TRY foi incluído para evitar a necessidade de vários IF, lendo sobre o porque isso retorna erro pelo o que entendi é que sempre que acontecer "algo inesperado" o código precisar tratar esses casos e não apenas ignorar, uma referencia https://stackoverflow.com/questions/21553327/why-is-except-pass-a-bad-programming-practice , mas aqui não precisamos do TRY porque já sabemos os casos em que existe um método especifico então um simples IF resolve, com isso LOG também volta a ficar limpo.
requests.post
without timeout. It could wait for a long timeAqui tem apenas uma questão que é saber se o valor 5 está razoável? Ou outro valor? Ou se isso deveria ser um parâmetro para permitir alterações?
cc @rvalyi @renatonlima @marcelsavegnago @mileo @antoniospneto @kaynnan