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

[9.0][MIG] l10n_es_dua_sii: Migration to 9.0 #847

Merged
merged 6 commits into from
May 9, 2018

Conversation

MouTio
Copy link
Member

@MouTio MouTio commented May 8, 2018

@pedrobaeza
Copy link
Member

No has respetado el historial de commits.

@MouTio
Copy link
Member Author

MouTio commented May 8, 2018

@pedrobaeza No sé a qué te refieres. El módulo no existía en la versión 9, así que lo he copiado y adaptado de la versión 10.

@pedrobaeza
Copy link
Member

Claro, y debes preservar los commits originales que introducen el código antes de que hagas los pequeños cambios para que funcione en v9. Puedes hacerlo con git cherry-pick o con git am

@MouTio MouTio closed this May 8, 2018
@MouTio
Copy link
Member Author

MouTio commented May 8, 2018

@pedrobaeza No sé cómo puedo hacer un cherry-pick de un directorio completo (l10n_es_dua_sii) en lugar de un commit en concreto. Estoy buscando pero no veo ninguna opción que lo haga.

@pedrobaeza
Copy link
Member

Utiliza https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-10.0#technical-method-to-migrate-a-module-from-90-to-100-branch, pero poniendo las ramas al revés

@MouTio MouTio reopened this May 8, 2018
@MouTio
Copy link
Member Author

MouTio commented May 8, 2018

Gracias @pedrobaeza
Ahora sí que sale el historial de commits.

@pedrobaeza pedrobaeza added this to the 9.0 milestone May 8, 2018
@pedrobaeza
Copy link
Member

Haz rebase ahora que está fusionado el otro módulo y además haz squash para juntar todos tus commits de migración.

@MouTio
Copy link
Member Author

MouTio commented May 8, 2018

Ya está.
Sin embargo sigue dando error en ci/runbot.
No entiendo por qué

@pedrobaeza
Copy link
Member

No has debido hacer el rebase, ya que no te coge el módulo l10n_es_dua.

@MouTio
Copy link
Member Author

MouTio commented May 9, 2018

Ya está el rebase correcto. Pensaba que haciendo el squash, el rebase se hacía automáticamente.

@@ -1,4 +1,4 @@
# -*- coding: utf-8 -*-
# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).

from . import models
from . import models
Copy link
Member

Choose a reason for hiding this comment

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

Por qué quitas de aquí la línea extra?

Copy link
Member Author

Choose a reason for hiding this comment

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

No la quité conscientemente. Quizás el editor me la quitó de forma automática.
Ahora luego cuando tenga un rato, restauro esa línea junto con la del otro fichero. (He visto que en el init.py de models también se ha quitado la línea extra.

Copy link
Member

Choose a reason for hiding this comment

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

Sí, revisa entonces tu IDE porque debería ser justo al revés (añadirla si no la tiene). Esto se hace para minimizar el diff en futuras adicciones: imagina que añades al final una nueva línea de código. El diff será de 2 líneas en lugar de una: la que es ahora penúltima, añadiendo el salto de línea, más la nueva.

Copy link
Member Author

Choose a reason for hiding this comment

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

No sabía el motivo por el que se dejaba la línea extra. Tiene todo el sentido del mundo :)

Ya he subido la versión con nueva línea.

@pedrobaeza pedrobaeza merged commit 3582b01 into OCA:9.0 May 9, 2018
@pedrobaeza pedrobaeza mentioned this pull request May 9, 2018
31 tasks
@MouTio MouTio deleted the mig_9.0_l10n_es_dua_sii branch May 9, 2018 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants