search

OCA / l10n-italy

Odoo Italian localization
https://www.odoo-italia.org
GNU Affero General Public License v3.0
149 stars 303 forks source link

Gestire i `position="replace"` nei vari moduli #3689

Open aleuffre opened 11 months ago

aleuffre commented 11 months ago

Modules

Describe the bug

Il nuovo linter OCA https://github.com/OCA/odoo-pre-commit-hooks consiglia fortemente di evitare i position="replace", e di usarli solo come ultima risorsa.

In https://github.com/OCA/l10n-italy/pull/3687 ho inserito

<!-- oca-hooks:disable=xml-dangerous-qweb-replace-low-priority -->

per disattivare il controllo su quel file. Questa però non è una soluzione ideale.

Soluzione consigliata qui https://github.com/OCA/sale-workflow/pull/2754#issuecomment-1785622763 da Pedro Baeza è di lasciare il check ma inserire priority="999" sulle view affette. Altre soluzioni ovviamente possono essere un refactor volto a eliminare completamente il position="replace"

To Reproduce

Affected versions:

SirAionTech commented 9 months ago

In https://github.com/OCA/l10n-italy/pull/3767 viene usato il nuovo linter e non ci sono problemi con position="replace" perché ovunque venga usato ha priorità 100, ad esempio in https://github.com/OCA/l10n-italy/blob/2f6d1837f05258e7af7d6dc4d8a2a393083303f2/l10n_it_fatturapa_out_wt/views/account_invoice_it_dati_ritenuta.xml#L66. Questa issue quindi si può considerare risolta per 16.0 o sarà risolta solo quando i position="replace" saranno rimossi?

aleuffre commented 9 months ago

Da indicazioni di Pedro Baeza, la priorità > 99 è una soluzione sufficiente. Marco la chiusura per 16.0

tafaRU commented 9 months ago

@aleuffre non sarebbe più corretto image attendere che #3767 venga mergiata prima di spuntare il flag?

SirAionTech commented 9 months ago

@aleuffre non sarebbe più corretto image attendere che #3767 venga mergiata prima di spuntare il flag?

:pray: Colpa mia che in https://github.com/OCA/l10n-italy/issues/3689#issuecomment-1853908431 ho scritto che poteva essere considerata risolta per 16.0. EDIT: In realtà quello che scrivevo è che il problema è già risolto in 16.0, non che sarà risolto dopo https://github.com/OCA/l10n-italy/pull/3767: le viste che hanno position="replace" già ora hanno priority > 99. Solo che ancora non c'è il controllo del nuovo linting. quando ci sarà (come succede ad esempio in https://github.com/OCA/l10n-italy/pull/3767) passerà senza bisogno di modifiche specifiche.

Invece parlando di flag, se ho ben capito per 14.0 c'è ancora da rimuovere i vari <!-- oca-hooks:disable=xml-dangerous-qweb-replace-low-priority --> quindi il flag non ci andrebbe, che ne pensi @aleuffre?

aleuffre commented 9 months ago

Invece parlando di flag, se ho ben capito per 14.0 c'è ancora da rimuovere i vari <!-- oca-hooks:disable=xml-dangerous-qweb-replace-low-priority --> quindi il flag non ci andrebbe, che ne pensi @aleuffre?

Io non l'ho cliccato quel flag... (cioè evidentemente l'ho cliccato io, probabilmente accidentalmente con il touchpad, ma...)

tafaRU commented 9 months ago

Io non l'ho cliccato quel flag.

su 14.0 l'avevo cliccato io pensando che fosse risolto :smiley:

github-actions[bot] commented 1 month ago

There hasn't been any activity on this issue in the past 6 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. If you want this issue to never become stale, please ask a PSC member to apply the "no stale" label.