Merge pull request OCA#65 from camptocamp/ref-mig-tips

MigrateAddon: rework generation of tips

oca_port/app.py

@@ -278,7 +278,7 @@ def _check_addon_exists(self, branch, raise_exc=False):
             if self.cli:
                 error = (
                     f"{bc.FAIL}{self.addon_path}{bc.ENDC} "
-                    "does not exist on {branch.ref()}"
+                    f"does not exist on {branch.ref()}"
                 )
             raise ValueError(error)
         return True

oca_port/migrate_addon.py

@@ -25,47 +25,50 @@
     "https://github.com/{from_org}/{repo_name}/compare/"
     "{to_branch}...{to_org}:{mig_branch}?expand=1&title={title}"
 )
-MIG_TIPS = "\n".join(
-    [
-        f"\n{bc.BOLD}{bc.OKCYAN}The next steps are:{bc.END}",
-        ("\t1) Reduce the number of commits " f"('{bc.DIM}OCA Transbot...{bc.END}'):"),
-        f"\t\t=> {bc.BOLD}{MIG_MERGE_COMMITS_URL}{bc.END}",
-        "\t2) Adapt the module to the {version} version:",
-        f"\t\t=> {bc.BOLD}" "{mig_tasks_url}" f"{bc.END}",
-        (
-            "\t3) On a shell command, type this for uploading the content to GitHub:\n"
-            f"{bc.DIM}"
-            "\t\t$ git add --all\n"
-            '\t\t$ git commit -m "[MIG] {addon}: Migration to {version}"\n'
-            "\t\t$ git push {remote} {mig_branch} --set-upstream"
-            f"{bc.END}"
-        ),
-        "\t4) Create the PR against {from_org}/{repo_name}:",
-        f"\t\t=> {bc.BOLD}" "{new_pr_url}" f"{bc.END}",
-    ]
-)
-BLACKLIST_TIPS = "\n".join(
-    [
-        f"\n{bc.BOLD}{bc.OKCYAN}The next steps are:{bc.END}",
-        (
-            "\t1) On a shell command, type this for uploading the content to GitHub:\n"
-            f"{bc.DIM}"
-            "\t\t$ git push {remote} {mig_branch} --set-upstream"
-            f"{bc.END}"
-        ),
-        "\t2) Create the PR against {from_org}/{repo_name}:",
-        f"\t\t=> {bc.BOLD}" "{new_pr_url}" f"{bc.END}",
-    ]
-)
-SIMPLIFIED_MIG_TIPS = "\n".join(
-    [
-        f"\n{bc.BOLD}{bc.OKCYAN}The next steps are:{bc.END}",
-        ("\t1) Reduce the number of commits " f"('{bc.DIM}OCA Transbot...{bc.END}'):"),
-        f"\t\t=> {bc.BOLD}{MIG_MERGE_COMMITS_URL}{bc.END}",
-        "\t2) Create the PR against {from_org}/{repo_name}:",
-        f"\t\t=> {bc.BOLD}" "{new_pr_url}" f"{bc.END}",
-    ]
-)
+MIG_STEPS = {
+    "reduce_commits": (
+        "Reduce the number of commits "
+        f"('{bc.DIM}OCA Transbot...{bc.END}'):\n"
+        f"\t\t=> {bc.BOLD}{MIG_MERGE_COMMITS_URL}{bc.END}"
+    ),
+    "adapt_module": (
+        "Adapt the module to the {version} version:\n"
+        f"\t\t=> {bc.BOLD}"
+        "{mig_tasks_url}"
+        f"{bc.END}"
+    ),
+    "amend_mig_commit": (
+        "Include your changes in the existing migration commit:\n"
+        f"{bc.DIM}"
+        "\t\t$ git add --all\n"
+        "\t\t$ git commit --amend\n"
+        "\t\t$ git push {remote} {mig_branch} --set-upstream"
+        f"{bc.END}"
+    ),
+    "commands": (
+        "On a shell command, type this for uploading the content to GitHub:\n"
+        f"{bc.DIM}"
+        "\t\t$ git add --all\n"
+        '\t\t$ git commit -m "[MIG] {addon}: Migration to {version}"\n'
+        "\t\t$ git push {remote} {mig_branch} --set-upstream"
+        f"{bc.END}"
+    ),
+    "create_pr": (
+        "Create the PR against {from_org}/{repo_name}:\n"
+        f"\t\t=> {bc.BOLD}"
+        "{new_pr_url}"
+        f"{bc.END}"
+    ),
+    "push_blacklist": (
+        "On a shell command, type this for uploading the content to GitHub:\n"
+        f"{bc.DIM}"
+        "\t\t$ git push {remote} {mig_branch} --set-upstream"
+        f"{bc.END}"
+    ),
+}
+MIG_USUAL_STEPS = ("reduce_commits", "adapt_module", "commands", "create_pr")
+MIG_BLACKLIST_STEPS = ("push_blacklist", "create_pr")
+MIG_ADAPTED_STEPS = ("reduce_commits", "adapt_module", "amend_mig_commit", "create_pr")
 
 
 class MigrateAddon(Output):
@@ -234,24 +237,31 @@ def _print_tips(self, blacklisted=False, adapted=False):
             title=pr_title_encoded,
         )
         if blacklisted:
-            tips = BLACKLIST_TIPS.format(
+            steps = self._generate_mig_steps(MIG_BLACKLIST_STEPS)
+            tips = steps.format(
                 from_org=self.app.upstream_org,
                 repo_name=self.app.repo_name,
                 remote=self.app.destination.remote,
                 mig_branch=self.mig_branch.name,
                 new_pr_url=new_pr_url,
             )
             print(tips)
-            return
+            return tips
         if adapted:
-            tips = SIMPLIFIED_MIG_TIPS.format(
+            steps = self._generate_mig_steps(MIG_ADAPTED_STEPS)
+            tips = steps.format(
                 from_org=self.app.upstream_org,
                 repo_name=self.app.repo_name,
+                version=self.app.target_version,
+                remote=self.app.destination.remote or "YOUR_REMOTE",
+                mig_branch=self.mig_branch.name,
+                mig_tasks_url=mig_tasks_url,
                 new_pr_url=new_pr_url,
             )
             print(tips)
-            return
-        tips = MIG_TIPS.format(
+            return tips
+        steps = self._generate_mig_steps(MIG_USUAL_STEPS)
+        tips = steps.format(
             from_org=self.app.upstream_org,
             repo_name=self.app.repo_name,
             addon=self.app.addon,
@@ -262,6 +272,14 @@ def _print_tips(self, blacklisted=False, adapted=False):
             new_pr_url=new_pr_url,
         )
         print(tips)
+        return tips
+
+    def _generate_mig_steps(self, steps):
+        result = []
+        for i, step in enumerate(steps, 1):
+            text = f"\t{i}) " + MIG_STEPS[step]
+            result.append(text)
+        return "\n".join(result)
 
     def _apply_code_pattern(self):
         print("Apply code pattern...")

oca_port/tests/test_app.py

@@ -95,6 +95,8 @@ def test_app_module_to_migrate(self):
         except SystemExit as exc:
             # exit code 100 means the module could be migrated
             self.assertEqual(exc.args[0], 100)
+
+    def test_app_module_does_not_exist(self):
         # The other way around, nothing to migrate as the module doesn't exist
         # (with CLI, the returned exit code is then 1)
         app = self._create_app(self.target2, self.source2)

oca_port/tests/test_migrate_addon.py

@@ -0,0 +1,33 @@
+from oca_port.migrate_addon import MigrateAddon
+
+from . import common
+
+
+class TestMigrateAddon(common.CommonCase):
+    def test_usual_tips(self):
+        app = self._create_app(self.source2, self.target2)
+        mig = MigrateAddon(app)
+        tips = mig._print_tips()
+        self.assertIn("1) Reduce the number of commits", tips)
+        self.assertIn("2) Adapt the module", tips)
+        self.assertIn("3) On a shell command", tips)
+        self.assertIn("4) Create the PR against", tips)
+        self.assertNotIn("5) ", tips)
+
+    def test_blacklist_tips(self):
+        app = self._create_app(self.source2, self.target2)
+        mig = MigrateAddon(app)
+        tips = mig._print_tips(blacklisted=True)
+        self.assertIn("1) On a shell command", tips)
+        self.assertIn("2) Create the PR against", tips)
+        self.assertNotIn("3) ", tips)
+
+    def test_adapted_tips(self):
+        app = self._create_app(self.source2, self.target2)
+        mig = MigrateAddon(app)
+        tips = mig._print_tips(adapted=True)
+        self.assertIn("1) Reduce the number of commits", tips)
+        self.assertIn("2) Adapt the module", tips)
+        self.assertIn("3) Include your changes", tips)
+        self.assertIn("4) Create the PR against", tips)
+        self.assertNotIn("5) ", tips)