Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes: > 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(). True. Lift the dying one callstack up, and make the lower level helper check_shared_symref() or something that returns NULL (ok) or that '/path/name' upon an error? Also I suspect that this comment will become hard to grok after the commit is actually made: > /* > - * $GIT_COMMON_DIR/HEAD is practically outside > + * $GIT_COMMON_DIR/$symref is practically outside > * $GIT_DIR so resolve_ref_unsafe() won't work (it > * uses git_path). Parse the ref ourselves. > */ A reviewer who is viewing both the pre- and post- text of the patch can see it used to say HEAD and now it is extended to $symref, but it would help to have "(e.g. HEAD)" after "...DIR/$symref", I think. -- 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