Run pre-commit if needed

OCA / odoo-module-migrator

Python library to migrate odoo base code from a version to another

GNU Affero General Public License v3.0

131 stars 119 forks source link

Run pre-commit if needed #25

Closed flachica closed 3 years ago

flachica commented 4 years ago

I think that can be better done. But it works for me. I don't know how it is packaged to test how it goes from the package

yelizariev commented 4 years ago

It this PR is about pre-commit, you may need to delete unrelated commits

flachica commented 4 years ago

I thought better not to include pre-commit. If it is not executed, you can see much better the changes made by odoo-module-migrator. Also, the programmer could execute pre-commit manually or automatically if he has the script installed

pedrobaeza commented 4 years ago

If I understand well how this library works, it takes the whole git work for preserving commit history and perform changes needed for the version, so the pre-commit should be interpolated in between. If not, you will have dirty changes that will overlap with pre-commit changes. Am I wrong?

flachica commented 4 years ago

You're right

If I understand well how this library works, it takes the whole git work for preserving commit history and perform changes needed for the version, so the pre-commit should be interpolated in between. If not, you will have dirty changes that will overlap with pre-commit changes. Am I wrong?

You're right

pedrobaeza commented 4 years ago

Then this is still needed and to perform the commit automatically after these changes, and then let as dirty the pure migration changes.

flachica commented 4 years ago

@yelizariev @pedrobaeza New push to delete unrelated commits

pedrobaeza commented 4 years ago

The commit history of this PR still seems to be a bit weird with 2 commits and a confusing commit message.

flachica commented 4 years ago

I hope was all correct

flachica commented 4 years ago

I pushed the suggested changes. I am fine with the change @ivantodorovich suggest to control odoo-module-migrator changes, but it may be that the commit history gets dirty. Do not you think?

ivantodorovich commented 4 years ago

I pushed the suggested changes. I am fine with the change @ivantodorovich suggest to control odoo-module-migrator changes, but it may be that the commit history gets dirty. Do not you think?

It's in the migration guideline to have precommit changes in a separate commit. This helps with the PR review, because you can easily find the actual changes done for the migration, separated from the pre-commit changes.

https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-13.0#technical-method-to-migrate-a-module-from-120-to-130-branch