Apply pre-commit and autosquash commit #41

sebastienbeau · 2020-11-13T13:35:44Z

@legalsylvain I think you are going to like it ;)
Based on recommandation here : https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-14.0#tasks-to-do-in-the-migration

README.rst

odoo_module_migrate/migration.py

sebastienbeau · 2020-11-14T07:46:07Z

@lmignon it may interest you too

hparfr

LGTM

legalsylvain

some comments inline. The rest LGTM. Nice addition.

Thanks a lot for your interest for this lib !

I imagine that many migrations are to be expected from the Akretion team ;-)

legalsylvain · 2020-12-02T13:34:44Z

setup.py

@@ -26,6 +26,7 @@
    install_requires=open('requirements.txt').read().splitlines(),
    entry_points=dict(console_scripts=[
        'odoo-module-migrate=odoo_module_migrate.__main__:main',
+        'odoo-module-squasher=odoo_module_migrate.squasher:main',


how ! you can use this new feature as a standalone, without migration process. nice !

In fact it's not exactly usable in stadalone, but I can do it by adding a new cmd

The current "odoo-module-squasher" is used as an arg to replace the TEXT editor for the git rebase

If you run

GIT_EDITOR=odoo-module-squasher GIT_SEQUENCE_EDITOR=odoo-module-squasher git rebase -i 14.0

Instead of have to edit with vim (or whatever text) my python script will do it.

It's the simpliest solution that I have found.

legalsylvain · 2020-12-02T13:38:53Z

odoo_module_migrate/migration.py

@@ -119,6 +135,13 @@ def _get_code_from_previous_branch(self, module_name, remote_name):
                'target': target_version,
                'module': module_name,
            }, path=self._directory_path)
+        if self._auto_squash:
+            logger.info("AutoSquash Commit")
+            _execute_shell(


Not sure to understand that point.

this will work if you didn't install the lib ? (if you just git clone and run locally your script ?)

why don't you call directly a function present in odoo_module_migrate/squasher.py instead of using shell ?

legalsylvain · 2020-12-02T13:40:53Z

odoo_module_migrate/migration.py

@@ -89,9 +90,24 @@ def __init__(
        for module_name in module_names:
            self._module_migrations.append(ModuleMigration(self, module_name))

+        if os.path.exists(".pre-commit-config.yaml"):
+            self._run_pre_commit(module_names)


maybe add an option for that point could avoid to force users to run pre-commit, if they don't want for any reason. could be default to True however.

don't you think ?

legalsylvain · 2020-12-02T13:41:36Z

README.rst

@@ -14,7 +14,7 @@ odoo-module-migrator

 ``odoo-module-migrator`` is a python3 library that allows you to realize automatically
 recurring changes when migrating Odoo modules from a version to another.
-for exemple: 
+for example:


I systematically make this mistake. I'm sorry (x4)

ivantodorovich

This looks interesting!! 🥳

ivantodorovich · 2020-12-03T13:14:26Z

README.rst

@@ -24,7 +24,8 @@ for exemple:

 This library will so:

-* (optionnaly) get commits from the old branch (if format-patch is enabled)
+* (optionnaly) get commits from the old branch and squash useless commit (if format-patch is enabled)


Suggested change

* (optionnaly) get commits from the old branch and squash useless commit (if format-patch is enabled)

* (optionally) get commits from the old branch and squash useless commits (if format-patch is enabled)

ivantodorovich · 2020-12-03T13:16:21Z

README.rst

+
+.. code-block:: shell
+
+    git config --global --add rebase.instructionFormat "<%ae> %s"


Can't this be added locally and automatically in the repo, if the feature is enabled?

ivantodorovich · 2020-12-03T13:36:55Z

odoo_module_migrate/squasher.py

+                        for regex in MATCHER.get(email, [])
+                        ]):
+                    logger.info("Squash commit %s" % line)
+                    lines.append(line.replace("pick", "squash", 1))


I need to test it but wouldn't this:

[IMP] Something [UPD] Update module.pot OCA Transbot updated translations from Transifex OCA Transbot updated translations from Transifex OCA Transbot updated translations from Transifex

result in this?:

[IMP] Something

What I would expect is this result:

[IMP] Something [UPD] Update module.pot OCA Transbot updated translations from Transifex

At least, that's what I normally do when squashing commits.. I group administrative commits together, but I don't mix them with the developer commits

Also I normally reorder lines as well:

Take this:

[IMP] Something [UPD] Update module.pot [IMP] Added extra feature OCA Transbot updated translations from Transifex OCA Transbot updated translations from Transifex [IMP] Another cool feature OCA Transbot updated translations from Transifex

I try to do this:

pick [IMP] Something pick [IMP] Added extra feature pick [IMP] Another cool feature pick [UPD] Update module.pot squash OCA Transbot updated translations from Transifex squash OCA Transbot updated translations from Transifex squash OCA Transbot updated translations from Transifex

The issue is not in the code but in my interpretation ! I was squashing administratif commit with the previous one

Squash administrative commits (if any) with the previous commit for reducing commit noise. They are named as "[UPD] README.rst", "[UPD] Update $MODULE.pot", "Update translation files" and similar names, and comes from OCA-git-bot, oca-travis or oca-transbot. IMPORTANT: Don't squash legit translation commits, authored by their translators, with the message "Translated using Weblate (...)".

I need some help

@pedrobaeza what do you think, how we should squash administrative commit ?

That's a difficult one, as sometimes the order is not the "real" one depending on the first date stamped on the commits, so I would leave this one not automated and subjected to the contributor criteria. There are already contributors that don't attend this when migrating modules. The idea behind is to have a cleaner commit history, but it's not mandatory.

Ok, regarding the best practice.
If I have a translation commit I should try to merge them with other translation commit ? or I should merge it into the previous one (that can be a real commit of code)

If I have a translation commit I should try to merge them with other translation commit ? or I should merge it into the previous one (that can be a real commit of code)

I'm not a git expert, but I think that the first option (merge translation commit) makes more sense.

Here's the wiki document with examples: https://github.com/OCA/maintainer-tools/wiki/Merge-commits-in-pull-requests

ivantodorovich · 2020-12-03T15:01:09Z

odoo_module_migrate/squasher.py

+            if line.startswith("#"):
+                count += 1
+            # we only commit the first commit message
+            # so we cut after the first commit message


If you only care about the first commit message, you should use fixup instead of squash.
It will do that automatically for you, and no need to edit any commit message

odoo_module_migrate/migration.py

yajo · 2020-12-14T11:05:32Z

README.rst

+The OCA recommand to squash automatic commit (from bot)
+This migration script will do it automatically but you need to configure
+correctly your git, so the script can apply the squash correctly
+
+You need to apply the following global config to your git
+
+.. code-block:: shell
+
+    git config --global --add rebase.instructionFormat "<%ae> %s"
+
+Then you can use the --auto-squash option


Suggested change

The OCA recommand to squash automatic commit (from bot)

This migration script will do it automatically but you need to configure

correctly your git, so the script can apply the squash correctly

You need to apply the following global config to your git

.. code-block:: shell

git config --global --add rebase.instructionFormat "<%ae> %s"

Then you can use the --auto-squash option

To use the OCA recommend to autosquash commits from bot, add ``--auto-squash``:

See below.

yajo · 2020-12-14T11:09:10Z

odoo_module_migrate/migration.py

+            _execute_shell(
+                "GIT_EDITOR=odoo-module-squasher "
+                "GIT_SEQUENCE_EDITOR=odoo-module-squasher "
+                "git rebase  -i %(target)s" % {"target": target_version}


This way, users don't need a specific instruction format configured in their Git:

Suggested change

"git rebase -i %(target)s" % {"target": target_version}

"git -c rebase.instructionFormat='<%%ae> %%s' rebase -i %(target)s" % {"target": target_version}

Will translate this: ```xml <div class="oe_button_box" name="button_box"> <button class="oe_stat_button" icon="fa-archive" name="toggle_active" type="object" > <field name="active" options='{"terminology": "archive"}' widget="boolean_button" /> </button> </div> ``` Into this: ```xml <div class="oe_button_box" name="button_box"> <field name="active" invisible="1" /> <widget name="web_ribbon" title="Archived" bg_color="bg-danger" attrs="{'invisible': [('active', '=', True)]}" /> </div> ``` Whitespace is bad formatted, but that's a job for OCA#41. @Tecnativa TT25940

joao-p-marques · 2020-12-16T15:29:49Z

LGTM, apart from the suggestions already made 👍

legalsylvain · 2022-01-03T10:37:16Z

Hi @sebastienbeau. happy new year.

could we move forward with this one. there are some conflict, and some unanswered comment.

thanks !

legalsylvain · 2022-01-03T10:40:21Z

/ocabot rebase

OCA-git-bot · 2022-01-03T10:40:29Z

@legalsylvain The rebase process failed, because command git rebase origin/master failed with output:

First, rewinding head to replay your work on top of it...
Applying: [IMP] run pre-commit if config exist
Using index info to reconstruct a base tree...
M	odoo_module_migrate/migration.py
M	odoo_module_migrate/tools.py
Falling back to patching base and 3-way merge...
Auto-merging odoo_module_migrate/tools.py
CONFLICT (content): Merge conflict in odoo_module_migrate/tools.py
Auto-merging odoo_module_migrate/migration.py
CONFLICT (content): Merge conflict in odoo_module_migrate/migration.py
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch' to see the failed patch
Patch failed at 0001 [IMP] run pre-commit if config exist
Resolve all conflicts manually, mark them as resolved with
"git add/rm <conflicted_files>", then run "git rebase --continue".
You can instead skip this commit: run "git rebase --skip".
To abort and get back to the state before "git rebase", run "git rebase --abort".

legalsylvain · 2023-05-15T10:50:57Z

With the creation of oca-port, it is interesting to keep this feature here. It seems that it is more in the scope of the oca-port. What do you think ?

sebastienbeau force-pushed the apply-prettier-and-autosquash-commit branch 3 times, most recently from 9831b19 to 102979a Compare November 13, 2020 14:26

yelizariev reviewed Nov 13, 2020

View reviewed changes

README.rst Outdated Show resolved Hide resolved

odoo_module_migrate/migration.py Outdated Show resolved Hide resolved

odoo_module_migrate/migration.py Outdated Show resolved Hide resolved

sebastienbeau added 2 commits November 13, 2020 17:40

[IMP] run pre-commit if config exist

9e03ec9

[IMP] squash automagically commit from bot

ba765bc

sebastienbeau force-pushed the apply-prettier-and-autosquash-commit branch from 102979a to ba765bc Compare November 13, 2020 16:46

fixup! [IMP] run pre-commit if config exist

5082e2f

sebastienbeau force-pushed the apply-prettier-and-autosquash-commit branch from 3984a98 to 4638101 Compare November 13, 2020 17:10

fixup! [IMP] squash automagically commit from bot

9eee0c1

sebastienbeau force-pushed the apply-prettier-and-autosquash-commit branch from 4638101 to 9eee0c1 Compare November 13, 2020 17:51

sebastienbeau changed the title ~~Apply prettier and autosquash commit~~ Apply pre-commit and autosquash commit Nov 14, 2020

hparfr approved these changes Dec 1, 2020

View reviewed changes

hparfr mentioned this pull request Dec 1, 2020

[WIP][14.0][MIG] shopfloor OCA/wms#90

Closed

[IMP] refactor the code to use regex

c5c5cf1

sebastienbeau force-pushed the apply-prettier-and-autosquash-commit branch from 9982684 to c5c5cf1 Compare December 2, 2020 13:35

legalsylvain reviewed Dec 2, 2020

View reviewed changes

[IMP] update readme

26694ed

ivantodorovich reviewed Dec 3, 2020

View reviewed changes

[FIX] fix issue if pre-commit do not change anything

3b590c8

yajo requested changes Dec 14, 2020

View reviewed changes

yajo mentioned this pull request Dec 15, 2020

[IMP] MIgrate toggle-active button to v13 #46

Merged

joao-p-marques approved these changes Dec 16, 2020

View reviewed changes

joao-p-marques mentioned this pull request Jul 13, 2021

Apply pre commit #55

Merged

legalsylvain added the need fixing label Jan 3, 2022

SirTakobi mentioned this pull request Mar 3, 2023

About squashing bot commits OCA/maintainer-tools#559

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Apply pre-commit and autosquash commit #41

Apply pre-commit and autosquash commit #41

sebastienbeau commented Nov 13, 2020

sebastienbeau commented Nov 14, 2020

hparfr left a comment

legalsylvain left a comment

legalsylvain Dec 2, 2020

sebastienbeau Dec 3, 2020 •

edited

Loading

legalsylvain Dec 2, 2020 •

edited

Loading

legalsylvain Dec 2, 2020

sebastienbeau Dec 3, 2020

legalsylvain Dec 2, 2020

ivantodorovich left a comment

ivantodorovich Dec 3, 2020

ivantodorovich Dec 3, 2020

ivantodorovich Dec 3, 2020

sebastienbeau Dec 4, 2020

pedrobaeza Dec 4, 2020

sebastienbeau Dec 4, 2020

legalsylvain Dec 4, 2020

ivantodorovich Dec 14, 2020

ivantodorovich Dec 3, 2020

yajo Dec 14, 2020 •

edited

Loading

This comment was marked as resolved.

yajo Dec 14, 2020

joao-p-marques commented Dec 16, 2020

legalsylvain commented Jan 3, 2022

legalsylvain commented Jan 3, 2022

OCA-git-bot commented Jan 3, 2022

legalsylvain commented May 15, 2023

	* (optionnaly) get commits from the old branch and squash useless commit (if format-patch is enabled)
	* (optionally) get commits from the old branch and squash useless commits (if format-patch is enabled)


		.. code-block:: shell

		git config --global --add rebase.instructionFormat "<%ae> %s"

	"git rebase -i %(target)s" % {"target": target_version}
	"git -c rebase.instructionFormat='<%%ae> %%s' rebase -i %(target)s" % {"target": target_version}

Apply pre-commit and autosquash commit #41

Are you sure you want to change the base?

Apply pre-commit and autosquash commit #41

Conversation

sebastienbeau commented Nov 13, 2020

sebastienbeau commented Nov 14, 2020

hparfr left a comment

Choose a reason for hiding this comment

legalsylvain left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sebastienbeau Dec 3, 2020 • edited Loading

Choose a reason for hiding this comment

legalsylvain Dec 2, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ivantodorovich left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yajo Dec 14, 2020 • edited Loading

Choose a reason for hiding this comment

This comment was marked as resolved.

Choose a reason for hiding this comment

joao-p-marques commented Dec 16, 2020

legalsylvain commented Jan 3, 2022

legalsylvain commented Jan 3, 2022

OCA-git-bot commented Jan 3, 2022

legalsylvain commented May 15, 2023

sebastienbeau Dec 3, 2020 •

edited

Loading

legalsylvain Dec 2, 2020 •

edited

Loading

yajo Dec 14, 2020 •

edited

Loading