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] [FIX] l10n_it_declaration_of_intent: use currency.compare_amounts() #3304

Conversation

TheMule71
Copy link
Contributor

In alcuni casi il confronto fallisce anche se i due float sono in teoria indentici.
Vd. https://discord.com/channels/753902328494424064/806815905006223411/1106128818642628730

Nota, l10n_it_declaration_of_intent usa Float invece di Monetary e non si porta dietro una currency. Per questo motivo uso la currency della fattura, assumento sia quella giusta (non potendo esserci un controllo di coerenza tra le due currency). In pratica si tratta sempre di EUR.

@TheMule71
Copy link
Contributor Author

@primes2h
Mi confermi il primo commit per cortesia?

@TheMule71 TheMule71 force-pushed the 14.0-fix-l10n_it_declaration_of_intent-compare-amount branch from 21cfc35 to 8fbae60 Compare May 12, 2023 12:14
@TheMule71 TheMule71 changed the title [14.0] [FIX] l10n_it_declaration_of_intent: use currency.compare_amount() [14.0] [FIX] l10n_it_declaration_of_intent: use currency.compare_amounts() May 12, 2023
@primes2h
Copy link
Contributor

@primes2h Mi confermi il primo commit per cortesia?

Perché hai eliminato il .pot?

@TheMule71
Copy link
Contributor Author

@primes2h Mi confermi il primo commit per cortesia?

Perché hai eliminato il .pot?

Perché quello vero è https://github.com/TheMule71/l10n-italy/blob/14.0-fix-l10n_it_declaration_of_intent-compare-amount/l10n_it_declaration_of_intent/i18n/l10n_it_declaration_of_intent.pot

io ho eliminato l10n_it_dichiarazione_intento.pot

@primes2h
Copy link
Contributor

@primes2h Mi confermi il primo commit per cortesia?

Perché hai eliminato il .pot?

Perché quello vero è https://github.com/TheMule71/l10n-italy/blob/14.0-fix-l10n_it_declaration_of_intent-compare-amount/l10n_it_declaration_of_intent/i18n/l10n_it_declaration_of_intent.pot

io ho eliminato l10n_it_dichiarazione_intento.pot

Ah, ok!
Allora forse sarebbe meglio usare "deprecated" al posto di "old" nel messaggio di commit, è più chiaro.

@TheMule71
Copy link
Contributor Author

@primes2h Mi confermi il primo commit per cortesia?

Perché hai eliminato il .pot?

Perché quello vero è https://github.com/TheMule71/l10n-italy/blob/14.0-fix-l10n_it_declaration_of_intent-compare-amount/l10n_it_declaration_of_intent/i18n/l10n_it_declaration_of_intent.pot
io ho eliminato l10n_it_dichiarazione_intento.pot

Ah, ok! Allora forse sarebbe meglio usare "deprecated" al posto di "old" nel messaggio di commit, è più chiaro.

È letteralmente il vecchio file (prima che il modulo venisse rinominato) che è rimasto in giro dai tempi della migrazione. Non è "deprecated", ci siamo solo dimenticati di cancellarlo.

@primes2h
Copy link
Contributor

@primes2h Mi confermi il primo commit per cortesia?

Perché hai eliminato il .pot?

Perché quello vero è https://github.com/TheMule71/l10n-italy/blob/14.0-fix-l10n_it_declaration_of_intent-compare-amount/l10n_it_declaration_of_intent/i18n/l10n_it_declaration_of_intent.pot
io ho eliminato l10n_it_dichiarazione_intento.pot

Ah, ok! Allora forse sarebbe meglio usare "deprecated" al posto di "old" nel messaggio di commit, è più chiaro.

È letteralmente il vecchio file (prima che il modulo venisse rinominato) che è rimasto in giro dai tempi della migrazione. Non è "deprecated", ci siamo solo dimenticati di cancellarlo.

@primes2h Mi confermi il primo commit per cortesia?

Perché hai eliminato il .pot?

Perché quello vero è https://github.com/TheMule71/l10n-italy/blob/14.0-fix-l10n_it_declaration_of_intent-compare-amount/l10n_it_declaration_of_intent/i18n/l10n_it_declaration_of_intent.pot
io ho eliminato l10n_it_dichiarazione_intento.pot

Ah, ok! Allora forse sarebbe meglio usare "deprecated" al posto di "old" nel messaggio di commit, è più chiaro.

È letteralmente il vecchio file (prima che il modulo venisse rinominato) che è rimasto in giro dai tempi della migrazione. Non è "deprecated", ci siamo solo dimenticati di cancellarlo.

Questo mi è chiaro. Il "deprecated" l'avevo inteso in un altro senso ma non c'è alcun problema.

Copy link
Contributor

@primes2h primes2h left a comment

Choose a reason for hiding this comment

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

LGTM, revisione tecnica.

@primes2h
Copy link
Contributor

P.S.: il problema si presenta anche nella 12.0 o nella 16.0?
In tal caso sarebbe il caso di aprire una issue per tenerne traccia.

@TheMule71
Copy link
Contributor Author

P.S.: il problema si presenta anche nella 12.0 o nella 16.0? In tal caso sarebbe il caso di aprire una issue per tenerne traccia.

12.0 no, perché il modulo aveva il nome italiano
16.0 sì

@primes2h
Copy link
Contributor

P.S.: il problema si presenta anche nella 12.0 o nella 16.0? In tal caso sarebbe il caso di aprire una issue per tenerne traccia.

12.0 no, perché il modulo aveva il nome italiano 16.0 sì

Mi riferivo al problema principale (rif. discord) risolto dal secondo commit.

@TheMule71
Copy link
Contributor Author

P.S.: il problema si presenta anche nella 12.0 o nella 16.0? In tal caso sarebbe il caso di aprire una issue per tenerne traccia.

12.0 no, perché il modulo aveva il nome italiano 16.0 sì

Mi riferivo al problema principale (rif. discord) risolto dal secondo commit.

In tal caso, sì:
12.0 (codice diverso ma stesso problema):


16.0 (stesso codice della 14):

@TheMule71 TheMule71 force-pushed the 14.0-fix-l10n_it_declaration_of_intent-compare-amount branch from 8fbae60 to 6212f04 Compare February 16, 2024 10:23
@TheMule71
Copy link
Contributor Author

Rebase dopo #3967

@sergiocorato
Copy link
Contributor

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 14.0-ocabot-merge-pr-3304-by-sergiocorato-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit f705bcf into OCA:14.0 Feb 16, 2024
7 checks passed
@OCA-git-bot
Copy link
Contributor

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

@TheMule71 TheMule71 deleted the 14.0-fix-l10n_it_declaration_of_intent-compare-amount branch February 16, 2024 11:32
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.

l10n_it_declaration_of_intent - bug in float comparisons
5 participants