git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Rubén Justo" <rjusto@gmail.com>
Cc: Git List <git@vger.kernel.org>, Phillip Wood <phillip.wood123@gmail.com>
Subject: Re: [PATCH v2 1/3] branch: fix die_if_checked_out() when ignore_current_worktree
Date: Sun, 22 Jan 2023 11:58:05 -0800	[thread overview]
Message-ID: <xmqqk01eqr3m.fsf@gitster.g> (raw)
In-Reply-To: 766b25e1-2d7a-7b5c-10a9-43e545a57dba@gmail.com

Rubén Justo <rjusto@gmail.com> writes:

>> As the proposed log message explained, updating die_if_checked_out()
>> with this patch would fix a bug---can we demonstrate the existing
>> breakage and protect the fix from future breakages by adding a test
>> or two?
>
> 2/3 and 3/3, I think makes more sense on its own commit.

Hmph, how so?  Especially once you split 1/3 into the preliminary
refactoring and the real fix, the fix becomes fairly small and
clear.  And the tests to protect the fix would go best in the same
commit.

>> The above comment is about find_shared_symref() which iterates over
>> worktrees and find the one that uses the named symref.  Now the
>> comment appears to apply to is_shared_symref() which does not
>> iterate but takes one specific worktree instance.  Do their
>> differences necessitate some updates to the comment?
>
> I think the comment still makes sense as is for the new function, both the
> description and the recommendation.  I will review it again.

OK.  Thanks.

>> > +int is_shared_symref(const struct worktree *wt, const char *symref,
>> > +		     const char *target)
>> > +{
>> 
>> What this function does sound more like "is target in use in this
>> particular worktree by being pointed at by the symref?"  IOW, I do
>> not see where "shared" comes into its name from.
>> 
>> "HEAD" that is tentatively detached while bisecting or rebasing the
>> "target" branch is still considered to point at the "target", so
>> perhaps symref_points_at_target() or something?
>
> I tried to maintain the terms as much as possible.  I'll think about the name
> you suggest.

When you did not change a thing in such a way that it does not
change the relationship between that thing and other things, it
makes perfect sense to keep the same term to refer to the thing.
Otherwise, once the thing starts playing different roles in the
world, there may be a better word to refer to the updated and
improved thing.

  reply	other threads:[~2023-01-22 19:58 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 [this message]
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
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=xmqqk01eqr3m.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=phillip.wood123@gmail.com \
    --cc=rjusto@gmail.com \
    /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).