Repository: VirtusLab/git-machete
Pull request #1266 (2024-06-10 10:38:10)

Abort `git{hub,lab} restack-{pr,mr}` for branch marked as `push=no` instead of just warning


Patch:
diff --git a/docs/man/git-machete.1 b/docs/man/git-machete.1
index abe6948..8600ccb 100644
--- a/docs/man/git-machete.1
+++ b/docs/man/git-machete.1
@@ -27,7 +27,7 @@ level margin: \\n[rst2man-indent\\n[rst2man-indent-level]]
 .\" new: \\n[rst2man-indent\\n[rst2man-indent-level]]
 .in \\n[rst2man-indent\\n[rst2man-indent-level]]u
 ..
-.TH "GIT-MACHETE" "1" "May 24, 2024" "" "git-machete"
+.TH "GIT-MACHETE" "1" "Jun 10, 2024" "" "git-machete"
 .SH NAME
 git-machete \- git-machete 3.26.1
 .sp
diff --git a/git_machete/client.py b/git_machete/client.py
index 6433171..abe6ebf 100644
--- a/git_machete/client.py
+++ b/git_machete/client.py
@@ -2290,56 +2290,55 @@ class MacheteClient:
         if s in statuses_to_push:
             if current_branch in self.annotations and not self.annotations[current_branch].qualifiers.push:
                 subcommand = "retarget-" + spec.pr_short_name.lower()
-                warn(f'Branch <b>{current_branch}</b> is marked as `push=no`; skipping the push.\n'
-                     f'Did you want to just use `git machete {spec.git_machete_command} {subcommand}`?\n')
+                raise MacheteException(
+                    f'Branch <b>{current_branch}</b> is marked as `push=no`; aborting the restack.\n'
+                    f'Did you want to just use `git machete {spec.git_machete_command} {subcommand}`?\n')
 
-                self.retarget_pr(spec, head, ignore_if_missing=False)
-            else:
-                converted_to_draft = code_hosting_client.set_draft_status_of_pull_request(pr.number, target_draft_status=True)
-                if converted_to_draft:
-                    print(f'{pr.display_text()} has been temporarily marked as draft')
+            converted_to_draft = code_hosting_client.set_draft_status_of_pull_request(pr.number, target_draft_status=True)
+            if converted_to_draft:
+                print(f'{pr.display_text()} has been temporarily marked as draft')
 
-                # Note that retarget should happen BEFORE push, see issue #1222
-                self.retarget_pr(spec, head, ignore_if_missing=False)
+            # Note that retarget should happen BEFORE push, see issue #1222
+            self.retarget_pr(spec, head, ignore_if_missing=False)
 
-                if s == SyncToRemoteStatuses.AHEAD_OF_REMOTE:
-                    assert remote is not None
-                    self.__handle_ahead_state(
-                        current_branch=current_branch,
-                        remote=remote,
-                        is_called_from_traverse=False,
-                        opt_push_tracked=True,
-                        opt_yes=True)
-                elif s == SyncToRemoteStatuses.UNTRACKED:
-                    self.__handle_untracked_state(
-                        branch=current_branch,
-                        is_called_from_traverse=False,
-                        is_called_from_code_hosting=True,
-                        opt_push_tracked=True,
-                        opt_push_untracked=True,
-                        opt_yes=True)
-                elif s == SyncToRemoteStatuses.DIVERGED_FROM_AND_NEWER_THAN_REMOTE:
-                    assert remote is not None
-                    self.__handle_diverged_and_newer_state(
-                        current_branch=current_branch,
-                        remote=remote,
-                        is_called_from_traverse=False,
-                        opt_push_tracked=True,
-                        opt_yes=True)
-                else:
-                    raise UnexpectedMacheteException(f"Invalid sync to remote status: {s}.")
+            if s == SyncToRemoteStatuses.AHEAD_OF_REMOTE:
+                assert remote is not None
+                self.__handle_ahead_state(
+                    current_branch=current_branch,
+                    remote=remote,
+                    is_called_from_traverse=False,
+                    opt_push_tracked=True,
+                    opt_yes=True)
+            elif s == SyncToRemoteStatuses.UNTRACKED:
+                self.__handle_untracked_state(
+                    branch=current_branch,
+                    is_called_from_traverse=False,
+                    is_called_from_code_hosting=True,
+                    opt_push_tracked=True,
+                    opt_push_untracked=True,
+                    opt_yes=True)
+            elif s == SyncToRemoteStatuses.DIVERGED_FROM_AND_NEWER_THAN_REMOTE:
+                assert remote is not None
+                self.__handle_diverged_and_newer_state(
+                    current_branch=current_branch,
+                    remote=remote,
+                    is_called_from_traverse=False,
+                    opt_push_tracked=True,
+                    opt_yes=True)
+            else:
+                raise UnexpectedMacheteException(f"Invalid sync to remote status: {s}.")
 
-                self.__print_new_line(False)
-                self.status(
-                    warn_when_branch_in_sync_but_fork_point_off=True,
-                    opt_list_commits=False,
-                    opt_list_commits_with_hashes=False,
-                    opt_squash_merge_detection=SquashMergeDetection.NONE)
-                self.__print_new_line(False)
+            self.__print_new_line(False)
+            self.status(
+                warn_when_branch_in_sync_but_fork_point_off=True,
+                opt_list_commits=False,
+                opt_list_commits_with_hashes=False,
+                opt_squash_merge_detection=SquashMergeDetection.NONE)
+            self.__print_new_line(False)
 
-                if converted_to_draft:
-                    code_hosting_client.set_draft_status_of_pull_request(prs[0].number, target_draft_status=False)
-                    print(f'{pr.display_text()} has been marked as ready for review again')
+            if converted_to_draft:
+                code_hosting_client.set_draft_status_of_pull_request(prs[0].number, target_draft_status=False)
+                print(f'{pr.display_text()} has been marked as ready for review again')
 
         else:
             if s == SyncToRemoteStatuses.BEHIND_REMOTE:


Test patch:
diff --git a/tests/test_github_restack_pr.py b/tests/test_github_restack_pr.py
index a3da57b..8f40f25 100644
--- a/tests/test_github_restack_pr.py
+++ b/tests/test_github_restack_pr.py
@@ -222,21 +222,40 @@ class TestGitHubRestackPR(BaseTest):
             .push()
             .commit()
         )
+
         body: str = \
             """
             master
                 feature push=no
             """
         rewrite_branch_layout_file(body)
-
-        assert_success(
+        assert_failure(
             ['github', 'restack-pr'],
             """
-            Warn: Branch feature is marked as push=no; skipping the push.
+            Branch feature is marked as push=no; aborting the restack.
             Did you want to just use git machete github retarget-pr?
+            """
+        )
 
+        body = \
+            """
+            master
+                feature
+            """
+        rewrite_branch_layout_file(body)
+        assert_success(
+            ['github', 'restack-pr'],
+            """
+            PR #15 has been temporarily marked as draft
             Base branch of PR #15 has been switched to master
             Description of PR #15 has been updated
+            Pushing feature to origin...
+
+              master (untracked)
+              |
+              o-feature *  PR #15 (some_other_user)
+
+            PR #15 has been marked as ready for review again
             """
         )
         pr = github_api_state.get_pull_by_number(15)
diff --git a/tests/test_gitlab_restack_mr.py b/tests/test_gitlab_restack_mr.py
index f9229c7..210cf84 100644
--- a/tests/test_gitlab_restack_mr.py
+++ b/tests/test_gitlab_restack_mr.py
@@ -226,23 +226,43 @@ class TestGitLabRestackMR(BaseTest):
             .push()
             .commit()
         )
+
         body: str = \
             """
             master
                 feature push=no
             """
         rewrite_branch_layout_file(body)
-
-        assert_success(
+        assert_failure(
             ['gitlab', 'restack-mr'],
             """
-            Warn: Branch feature is marked as push=no; skipping the push.
+            Branch feature is marked as push=no; aborting the restack.
             Did you want to just use git machete gitlab retarget-mr?
+            """
+        )
 
+        body = \
+            """
+            master
+                feature
+            """
+        rewrite_branch_layout_file(body)
+        assert_success(
+            ['gitlab', 'restack-mr'],
+            """
+            MR !15 has been temporarily marked as draft
             Target branch of MR !15 has been switched to master
             Description of MR !15 has been updated
+            Pushing feature to origin...
+
+              master (untracked)
+              |
+              o-feature *  MR !15 (some_other_user)
+
+            MR !15 has been marked as ready for review again
             """
         )
+
         mr = gitlab_api_state.get_mr_by_number(15)
         assert mr is not None
         assert mr['draft'] is False