git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Rubén Justo" <rjusto@gmail.com>
To: phillip.wood@dunelm.org.uk, "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: Git List <git@vger.kernel.org>, Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH v3 3/4] rebase: refuse to switch to a branch already checked out elsewhere (test)
Date: Wed, 8 Feb 2023 01:43:45 +0100	[thread overview]
Message-ID: <19e3e0d9-406e-ac2f-c43a-b4e994035529@gmail.com> (raw)
In-Reply-To: <f4be3c97-eb15-790e-9746-96d0c9bbc5a7@dunelm.org.uk>

On 07-feb-2023 10:52:39, Phillip Wood wrote:
> Hi Rubén
> 
> On 06/02/2023 23:16, Rubén Justo wrote:
> > On 06-feb-2023 17:59:11, Ævar Arnfjörð Bjarmason wrote:
> > 
> > > > Let's add a test to notice if this changes in the future.
> > > 
> > > I for one would find this series much easier to follow if you started
> > > with this test, possibly with a test_expect_failure, and as we fix the
> > > relevant code flip them (both this and the subsequent one) to run
> > > successfully, and include them as part of the commit that fixes the
> > > bug).
> > > 
> > > Maybe there's reasons for why that's tricky to do in this case, so
> > > please ignore this if so.
> > 
> > I'll give it try, I like the idea.  Thanks.
> 
> Squashing the last three commits together so that the tests are introduced
> in the same commit as the fix as Junio suggested in his comments on the
> previous round would be very welcome.

Yes, I considered that, but I think that keeping the tests in their own
commit is reasonable.  The tests protect against a malfunction that we
did not notice when the implementation was done.  The commits reference
(and use similar subjects) to the original commit where we should have
introduced the tests.  If the tests fail, I think it will be easier and
less confusing to reach the original commit where the implementation was
done if we keep them separated, rather than combining all three commits.

I'm going to reorder the commits and change to use test_expect_failure().
This way the commit with the fix will also be linked.

  reply	other threads:[~2023-02-08  0:43 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-17  0:36 [PATCH] worktree: teach find_shared_symref to ignore current worktree Rubén Justo
2023-01-17 23:27 ` Junio C Hamano
2023-01-18 23:50   ` Rubén Justo
2023-01-19 10:48     ` Phillip Wood
2023-01-19 23:18       ` Rubén Justo
2023-01-22  1:20 ` [PATCH v2 0/3] fix die_if_checked_out() when ignore_current_worktree Rubén Justo
2023-01-22  1:23   ` [PATCH v2 1/3] branch: " Rubén Justo
2023-01-22  1:50     ` Junio C Hamano
2023-01-22 11:51       ` Rubén Justo
2023-01-22 19:58         ` Junio C Hamano
2023-01-22 23:21           ` Rubén Justo
2023-01-24 10:35             ` Phillip Wood
2023-01-26  3:07               ` Rubén Justo
2023-01-22  1:28   ` [PATCH v2 2/3] rebase: refuse to switch to a branch already checked out elsewhere (test) Rubén Justo
2023-01-22  1:28   ` [PATCH v2 3/3] switch: reject if the branch is " Rubén Justo
2023-02-04 23:19   ` [PATCH v3 0/4] fix die_if_checked_out() when ignore_current_worktree Rubén Justo
2023-02-04 23:25     ` [PATCH v3 1/4] worktree: introduce is_shared_symref() Rubén Justo
2023-02-07 10:44       ` Phillip Wood
2023-02-04 23:25     ` [PATCH v3 2/4] branch: fix die_if_checked_out() when ignore_current_worktree Rubén Justo
2023-02-06 16:56       ` Ævar Arnfjörð Bjarmason
2023-02-06 23:09         ` Rubén Justo
2023-02-07 10:50         ` Phillip Wood
2023-02-07 12:58           ` Ævar Arnfjörð Bjarmason
2023-02-04 23:26     ` [PATCH v3 3/4] rebase: refuse to switch to a branch already checked out elsewhere (test) Rubén Justo
2023-02-06 16:59       ` Ævar Arnfjörð Bjarmason
2023-02-06 23:16         ` Rubén Justo
2023-02-07 10:52           ` Phillip Wood
2023-02-08  0:43             ` Rubén Justo [this message]
2023-02-08  5:19               ` Junio C Hamano
2023-02-08 22:09                 ` Rubén Justo
2023-02-04 23:26     ` [PATCH v3 4/4] switch: reject if the branch is " Rubén Justo
2023-02-15  4:17       ` Eric Sunshine
2023-02-15 22:17         ` Rubén Justo
2023-02-25 14:14   ` [PATCH v4 0/4] fix die_if_checked_out() when ignore_current_worktree Rubén Justo
2023-02-25 14:21     ` [PATCH v4 1/4] worktree: introduce is_shared_symref() Rubén Justo
2023-02-25 14:22     ` [PATCH v4 2/4] branch: fix die_if_checked_out() when ignore_current_worktree Rubén Justo
2023-02-25 14:22     ` [PATCH v4 3/4] rebase: refuse to switch to a branch already checked out elsewhere (test) Rubén Justo
2023-02-25 14:22     ` [PATCH v4 4/4] switch: reject if the branch is " Rubén Justo
2023-02-25 22:50     ` [PATCH v4 0/4] fix die_if_checked_out() when ignore_current_worktree Junio C Hamano
2023-02-27  0:00       ` Rubén Justo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=19e3e0d9-406e-ac2f-c43a-b4e994035529@gmail.com \
    --to=rjusto@gmail.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=phillip.wood@dunelm.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).