-
-
Notifications
You must be signed in to change notification settings - Fork 306
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
[MIG] l10n_it_accompanying_invoice: Migration to 16.0 #4321
base: 16.0
Are you sure you want to change the base?
[MIG] l10n_it_accompanying_invoice: Migration to 16.0 #4321
Conversation
Hi @MarcoCalcagni, @aleuffre, @renda-dev, |
2fe42be
to
6148353
Compare
@SirAionTech per testare le migrazioni con OpenUpgrade fai una migrazione reale (in questo caso da 12 a 16) o ci sono dei metodi per simularle ed evitare tutti questi passaggi? |
6148353
to
ace8042
Compare
Creo un DB Visto che il nuovo commit di migrazione ace8042 è diverso dal mio 37862c6 (lì c'era anche la rinomina), ti puoi mettere come co-autore? |
/ocabot migration l10n_it_accompanying_invoice |
02faabe
to
47c2e8d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test funzionale: OK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This PR has the |
Vorrei precisare che seppur abbia seguito alla lettera la guida di OpenUpgrade, nei test che ho fatto la migrazione sembra non portare i dati alla versione 16. |
Magari aggiungi un commento con queste informazioni? |
o nel known issues |
@eLBati preferirei commentare del tutto ed inserire il perché nei know Issues come dice @francesco-ooops: basta commentare questa riga https://github.com/OCA/l10n-italy/pull/4321/files#diff-3cf0873429c1a4ad9a6fb27ba09151f46badea3db698f511640aac70c8239757R29? |
Prendo spunto da questo e metto il commento nella roadmap: https://github.com/OCA/l10n-italy/blob/16.0/l10n_it_riba/readme/ROADMAP.md |
6318820
to
cf607af
Compare
@eLBati @francesco-ooops ditemi se ora va bene |
@eLBati @francesco-ooops gentile reminder 🙏 |
@odooNextev ciao, non ho obiezioni, le review funzionali ci sono già :) |
@MarcoCalcagni @aleuffre @renda-dev potete fare il merge se non c'è nulla in contrario? |
@odooNextev Non posso mergiare, sono maintainer solo di |
Giusto, non avevo capito subito. |
Sì, più in generale basta un @OCA/local-italy-maintainers |
@SirAionTech hai ragione, ma ti citavo perchè avevi cominciato tu la migrazione di questo modulo (fondamentalmente ho solo apportato piccole correzioni e disattivato lo script di migrazione) e quindi sei sul pezzo. |
Visto che non supportiamo più la 12, non ci interessa automatizzare il porting dei dati. Però possiamo scrivere nel README che se qualcuno usa il modulo sulla 12, per non perdere dati può togliere la dipendenza da |
A mixin for fields and methods, a report for printing them
…10n_it_accompanying_invoice)
Co-authored-by: Simone Rubino <[email protected]> Co-authored-by: OdooNextev <[email protected]>
cf607af
to
acc80a1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eLBati va bene?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@odooNextev grazie. Non avevamo detto di togliere del tutto gli script di upgrade?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sinceramente non ricordo, però a quel punto togliere solo la dipendenza dal manifest obbligherebbe l'utente a fare tutto manualmente, invece gli script gli avrebbero dato qualche indizio se non portare quasi tutto.
Lascio all'utente il compito?
@LorenzoC0 puoi fare review a questa e poi backportiamo alla 14? |
Supersede #3742