Re: [PATCH v3 1/2] worktrees: add die_if_shared_symref

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jul 31, 2015 at 5:15 PM, David Turner <dturner@xxxxxxxxxxxxxxxx> wrote:
> On Fri, 2015-07-31 at 15:35 -0400, Eric Sunshine wrote:
>> On Fri, Jul 31, 2015 at 3:01 PM, David Turner <dturner@xxxxxxxxxxxxxxxx> wrote:
>> > Add a new function, die_if_shared_symref, which works like
>> > die_if_checked_out, but for all references.  Refactor
>> > die_if_checked_out to work in terms of die_if_shared_symref.
>> >
>> > Soon, we will use die_if_shared_symref to protect notes merges in
>> > worktrees.
>>
>> I wonder if the diagnostic:
>>
>>     'blorp' is already checked out at '/path/name/'
>>
>> emitted by check_linked_checkout() needs to be adjusted for this
>> change. It still makes sense for die_if_checked_out(), but sounds odd
>> for die_if_shared_symref().
>
> How about:
>
> 'refs/notes/y' is already referenced from 'NOTES_MERGE_REF' in
> '/home/dturner/git/t/trash directory.t3320-notes-merge-worktrees/'
>
> Does that make sense?

That might be the best we can do for the generic case of
die_if_shared_symref(), but I wonder how easily the typical user would
understand it when trying to checkout a branch already checked out
elsewhere. One concern is that that message almost comes across as an
internal Git error (thus inscrutable), whereas:

    % git checkout blorp
    'blorp' is already checked out at '/some/path/'

seems (to me) pretty clearly a user error, thus more easily understood.

> It's not the only place in the error messages that mentions
> NOTES_MERGE_REF, so I think we expect users to understand
> NOTES_MERGE_REF.  The alternative is to move the error handling to an
> even higher level so we can give a notes-specific message.  I don't
> think that's necessary, but I'll do it if others do.

Given such precedence, the generic error message might indeed be fine
for the notes merge case, however, in general, you'd probably get
better and more understandable error messages by tailoring them at the
call sites. (That's a positive vote from me for for lifting error
handling to a higher level.)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]