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

[MIG] l10n_it_accompanying_invoice: Migration to 16.0 #4321

Open
wants to merge 6 commits into
base: 16.0
Choose a base branch
from

Conversation

odooNextev
Copy link
Contributor

Supersede #3742

@OCA-git-bot
Copy link
Contributor

Hi @MarcoCalcagni, @aleuffre, @renda-dev,
some modules you are maintaining are being modified, check this out!

@stenext stenext force-pushed the 16.0-mig-l10n_it_accompanying_invoice branch from 2fe42be to 6148353 Compare August 8, 2024 07:32
@odooNextev
Copy link
Contributor Author

@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?
Per ora ho modificato gli script solo in maniera teorica senza testarli.

@SirAionTech
Copy link
Contributor

@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? Per ora ho modificato gli script solo in maniera teorica senza testarli.

Creo un DB 12.0, ci metto qualche dato a mano e faccio la migrazione fino a 16.0.

Visto che il nuovo commit di migrazione ace8042 è diverso dal mio 37862c6 (lì c'era anche la rinomina), ti puoi mettere come co-autore?

@SirAionTech
Copy link
Contributor

/ocabot migration l10n_it_accompanying_invoice

@OCA-git-bot OCA-git-bot added this to the 16.0 milestone Aug 19, 2024
@OCA-git-bot OCA-git-bot mentioned this pull request Aug 19, 2024
79 tasks
@stenext stenext force-pushed the 16.0-mig-l10n_it_accompanying_invoice branch from 02faabe to 47c2e8d Compare August 26, 2024 09:23
@stenext
Copy link

stenext commented Aug 26, 2024

Creo un DB 12.0, ci metto qualche dato a mano e faccio la migrazione fino a 16.0.

Ok, allora è come faccio io, ma è tedioso.

Visto che il nuovo commit di migrazione ace8042 è diverso dal mio 37862c6 (lì c'era anche la rinomina), ti puoi mettere come co-autore?

Fatto.

Copy link

@MaurizioPellegrinet MaurizioPellegrinet left a comment

Choose a reason for hiding this comment

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

Test funzionale: OK

Copy link
Contributor

@andreampiovesana andreampiovesana left a comment

Choose a reason for hiding this comment

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

LGTM

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@odooNextev
Copy link
Contributor Author

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.
Dobbiamo decidere se serve investire tempo nella migrazione di un modulo che alla 14.0 non era neanche stato portato, se lasciare comunque gli script come base di partenza nel caso qualcuno più avanti voglia riprenderli o cancellarli del tutto.

@eLBati
Copy link
Member

eLBati commented Oct 9, 2024

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. Dobbiamo decidere se serve investire tempo nella migrazione di un modulo che alla 14.0 non era neanche stato portato, se lasciare comunque gli script come base di partenza nel caso qualcuno più avanti voglia riprenderli o cancellarli del tutto.

Magari aggiungi un commento con queste informazioni?

@francesco-ooops
Copy link
Contributor

o nel known issues

@odooNextev
Copy link
Contributor Author

odooNextev commented Oct 10, 2024

@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?
In test non ho avuto problemi collaterali causati da questa migrazione, ma non porta i dati, però non vorrei causarne ad altri.
Allo stesso tempo non vorrei buttare via il lavoro mio e di @SirAionTech e chi vuole può sistemarlo partendo da una base.

@odooNextev
Copy link
Contributor Author

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

@stenext stenext force-pushed the 16.0-mig-l10n_it_accompanying_invoice branch from 6318820 to cf607af Compare October 11, 2024 13:17
@odooNextev
Copy link
Contributor Author

@eLBati @francesco-ooops ditemi se ora va bene

@odooNextev
Copy link
Contributor Author

@eLBati @francesco-ooops gentile reminder 🙏

@francesco-ooops
Copy link
Contributor

@odooNextev ciao, non ho obiezioni, le review funzionali ci sono già :)

@odooNextev
Copy link
Contributor Author

@MarcoCalcagni @aleuffre @renda-dev potete fare il merge se non c'è nulla in contrario?

@aleuffre
Copy link
Contributor

@odooNextev Non posso mergiare, sono maintainer solo di l10n_it_delivery_note

@odooNextev
Copy link
Contributor Author

@odooNextev Non posso mergiare, sono maintainer solo di l10n_it_delivery_note

Giusto, non avevo capito subito.
Allora devo aspettare @SirAionTech o @eLBati per esempio.

@SirAionTech
Copy link
Contributor

@odooNextev Non posso mergiare, sono maintainer solo di l10n_it_delivery_note

Giusto, non avevo capito subito. Allora devo aspettare @SirAionTech o @eLBati per esempio.

Sì, più in generale basta un @OCA/local-italy-maintainers

@odooNextev
Copy link
Contributor Author

@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.
@eLBati invece penso lo abbia proprio sviluppato.

@eLBati
Copy link
Member

eLBati commented Nov 15, 2024

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 l10n_it_ddt e poi preoccuparsi di spostare i dati nelle colonne giuste

@stenext stenext force-pushed the 16.0-mig-l10n_it_accompanying_invoice branch from cf607af to acc80a1 Compare November 15, 2024 14:52
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@eLBati va bene?

Copy link
Member

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?

Copy link
Contributor Author

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?

@francesco-ooops
Copy link
Contributor

@LorenzoC0 puoi fare review a questa e poi backportiamo alla 14?

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.