[17.0][MIG] l10n_es_igic: Migration to version 17.0 #3679
Conversation
|
Thanks for the contribution. Please preserve commit history following technical method explained in https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-17.0. If the jump is between several versions, you have to modify the source branch in the main command to accommodate it to this circumstance. |
Hola Pedro, no te entendí bien. He de hacer alguna cosa adicional? |
|
Hola, Moisés, no has seguido la guía de migración para preservar el historial de commits. Mira esa guía, y tienes que seguir exactamente esos pasos. En tu PR hay un solo commit, y estás "robando" la atribución del trabajo inicial del módulo. |
Currently translated at 100.0% (14 of 14 strings) Translation: l10n-spain-16.0/l10n-spain-16.0-l10n_es_igic Translate-URL: https://translation.odoo-community.org/projects/l10n-spain-16-0/l10n-spain-16-0-l10n_es_igic/es/
1493199
to
09e08a5
Compare
|
Hola Pedro, ya está arreglado el historial de commits |
|
Genial, enhorabuena! Echa por favor un vistazo a pre-commit: https://github.com/OCA/maintainer-tools/wiki/Install-pre-commit /ocabot migration l10n_es_igic |
|
He hecho un commit para los arreglos del pre-commit. Sale un error con pylint, pero entiendo como dice la documentación de que debo ignorarlo. Si no me equivoco estaría todo ok. Gracias @pedrobaeza por guiarme ;) |
|
Hola, Moisés, pre-commit aún se queja: Esto será porque estás haciendo múltiples archivos para heredar de la misma clase. Hazlos todos en una sola. Por otro lado, haz squash de estos fixes en el commit de la migración, no como commits aparte. Cuando ya esté todo bien, revisores de allí de Canarias deberán confirmar que todo está correcto para proceder con la fusión. |
fbc8828
to
9bd4e27
Compare
|
Hola @pedrobaeza, ya he arreglado el pre-commit. Además he hecho squash para tener un solo commit. Lo único que veo que fallan un par de test, los de codecov que no se muy bien que es, el resto en principio está correcto. |
|
Las de Codecov no son obligatorias. Indican que ahora la cantidad de código cubierta es menor, pero a veces tampoco es cierto (falla el sistema), y en este caso, no impacta. |
|
Perfecto @pedrobaeza :) entonces ya estaría, solo a falta de que los revisores den el visto bueno. |
|
Hola @mfargnoli, hoy hablamos por teléfono, gracias por la llamada. No soy revisor "oficial" pero estoy echando un vistazo a tu pull request y me ha llamado la atención que las importaciones de datos están comentadas en el manifiesto (ver manifest.py#L23). ¿Es intencional? |
|
Si @elcapo, decidí dejarlo así como referencia porque viene así del módulo del igic de la v16 y en la v17 cambia bastante de como hacerlo. Realmente se podría eliminar. Por otro lado y si preguntas por líneas comentadas, si miras en el template, verás que hay dos entradas para impuestos y planes de contablidad para las cooperativas, ya que la v17 introduce este tipo de contabilidad para estás entidades, que en la v16 no existen. De esta manera lo dejo preparado para poder añadirlos, pero como desconozco los planes de contabilidad y los impuestos que les corresponden para Canarias y entiendo que no es parte de la migración lo he dejado pendiente para un posible añadido. |
|
Una pregunta @pedrobaeza, veo que pone que no hay conflictos con la rama base, pero que solo aquellos con permiso de escritura pueden hacer merge al pull request. He de añadir a "OCA Transbot" para ello? |
|
Uhm, no entiendo qué tiene que ver "OCA Transbot" en tu frase inicial, pero es correcto que tú no puedas fusionar, porque eso lo tiene que hacer una persona autorizada cuando se cumplan las condiciones de las líneas guía OCA. Sobre lo otro que comentaba Carlos, debes eliminar los archivos y las líneas comentadas. El histórico de cambios ya está ahí. Para eso existe el control de cambios git. |
|
Perfecto @pedrobaeza me pongo a quitar los comentarios. |
|
Sin ser una revisión oficial, he podido instalar los cambios en una Odoo 17.0-20240730 (Edición Community) y he podido configurar facturas con IGIC sin ningún problema. Gracias @mfargnoli por la contribución. Deseando que pase la revisión para empezar a usarla en producción. |
9bd4e27
to
93b606e
Compare
l10n_es_igic/readme/INSTALL.md
Outdated
| En caso contrario: | ||
|
|
||
| - Instalar este módulo. | ||
| - Instalar account_chart_update (OCA account-financial-tools) |
There was a problem hiding this comment.
Ahora no sería necesario este paso, ¿verdad?
There was a problem hiding this comment.
Si, correcto no haría falta
b691e73
to
ab3b74b
Compare
readme.md corregido readme corregido
ab3b74b
to
00a39e9
Compare
Ya he realizado la separación de los commits en |
|
Hey, thanks for contributing! Proceeding to merge this for you. |
|
Congratulations, your PR was merged at 5ff157e. Thanks a lot for contributing to OCA. ❤️ |
|
@pedrobaeza parece que hay algún fallo en algún porcentaje del igic. Lo corrijo y lo subo con un squash a la migración actual? |
|
@mfargnoli ya te acabo de hacer ping en el PR de otro contribuidor que lo ha corregido. Revisa y da el aprobado si lo ves correcto. |
No description provided.