Re: [PATCH] notes: accept any ref for merge

[Junio C Hamano <gitster@pobox.com>]

Johan Herland <johan@herland.net> writes:

> On Fri, Sep 19, 2014 at 11:39 AM, Jeff King <peff@peff.net> wrote:
>> On Fri, Sep 19, 2014 at 09:39:45AM +0200, Scott Chacon wrote:
>>> Currently if you try to merge notes, the notes code ensures that the
>>> reference is under the 'refs/notes' namespace. In order to do any sort
>>> of collaborative workflow, this doesn't work well as you can't easily
>>> have local notes refs seperate from remote notes refs.
>>>
>>> This patch changes the expand_notes_ref function to check for simply a
>>> leading refs/ instead of refs/notes to check if we're being passed an
>>> expanded notes reference. This would allow us to set up
>>> refs/remotes-notes or otherwise keep mergeable notes references outside
>>> of what would be contained in the notes push refspec.
>>
>> I think this change affects not just "git notes merge", but all of the
>> notes lookups (including just "git notes show")....
> ...
> Additionally, I suggest adding another test demonstrating your use
> case as well. Something like setting up a small scenario for notes
> collaboration, and walking through the various steps:
>
>  - Creating a couple of repos where notes are added/edited
>  - Setting up config to allow pushing and/or fetching notes
>  - Performing the push/fetch
>  - Merging with the corresponding local notes ref

Is it our future direction to set up refs/remote-notes/<remote>/
namespace?  If so, let's not do it piecemeail in an unorganized
guerrilla fashion by starting with a stealth enabler with an
associated test.  We risk not following through and leave the
resulting user experience more puzzling if we go that way.

By "stealth enabler" I mean the removal of refs/notes/ restriction
that was originally done as a safety measure to avoid mistakes of
storing notes outside.  The refs/remote-notes/ future direction
declares that it is no longer a mistake to store notes outside
refs/notes/, but that does not necessarily have to mean that
anywhere under refs/ is fine.  It may make more sense to be explicit
with the code touched here to allow traditional refs/notes/ and the
new hierarchy only.  That way, we will still keep the "avoid
mistakes" safety and enable the new layout at the same time.

The most important first step for that to happen is to make sure we
are on the same page on that future direction.  I personally think
refs/remote-notes/<remote> that runs parallel to the remote tracking
branch hierarchy refs/remotes/<remote> is a reasonable way to do
this, but my words are no way final.

Assuming that this is we all agree to go in that direction, let's
make a list of things to be done to codify it, and do them.  For a
starter, I think these are needed, perhaps?

 - This patch (or an enhancement to keep some safety)

 - Documentation updates to "git notes"

 - Documentation updates to Documentation/gitrepository-layout.txt

 - Update to "git clone" and "git remote add" to add a fetch refspec
   refs/notes:refs/remote-refs/<remote>/*

 - New tests you suggest