git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Kyle J. McKay" <mackyle@gmail.com>
To: Git mailing list <git@vger.kernel.org>
Cc: Scott Chacon <schacon@gmail.com>,
	Junio C Hamano <gitster@pobox.com>,
	Johan Herland <johan@herland.net>, Jeff King <peff@peff.net>
Subject: [PATCH v2 0/2] Accept any notes ref
Date: Mon,  5 Jan 2015 22:03:51 -1000	[thread overview]
Message-ID: <d4509363c8f670483dacdd2a5070f5a@74d39fa044aa309eaea14b9f57fe79c> (raw)

This is a resend of the original patch by Scott Chacon combined with the
suggested patch by Johan Herland with a summary of the discussion so far
in the hope to restart discussion about including this change.

The original thread can be found at [1].

The history so far:

The Patch 1/2 was originally submitted to allow using refs outside of
the refs/notes/ namespace as notes refs for the purpose of merging notes.
However, that patch actually just relaxes the restriction on notes refs so
that a fully-qualified ref will be used as-is in the notes code and affects
more than just notes merging.

The concern was then raised that this is a "stealth-enabler" patch and that
even if notes refs are allowed outside of refs/notes/ then they should still
be restricted to a some subset (to be determined) of the the refs/ hierarchy
and not just allowed anywhere.

But, I feel that the rest of Git does not restrict the user in this way -- if
the user really wants to do something that is "not recommended" there is
almost always a mechanism and/or option to leave the user in control and allow
it despite any recommendation(s) to the contrary.

So I am resending this summary with attached patches (both of which are
very trivial) to allow using any ref for notes as long as it's fully qualified
(i.e. starts with "refs/").  Patch 1/2 does that and patch 2/2 fixes the test
that breaks because of it.

In the hope of restarting discussion towards enabling use of refs outside of
refs/notes/ with the notes machinery I conclude by including Peff's final
reply to the original thread which I think contains the most compelling
aregument for inclusion:

On Dec 4, 2014, at 02:26, Jeff King wrote:

> On Sat, Nov 22, 2014 at 10:04:57AM -0800, Kyle J. McKay wrote:
>
>> On Sep 19, 2014, at 11:22, Junio C Hamano wrote:
>>
>>> 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.
>>
>> This is the part where I want to lobby for inclusion of this change.
>> I do not believe it is consistent with existing Git practice to
>> enforce restrictions like this (you can only display/edit/etc. notes
>> under refs/notes/*).
>
> Yeah, this is the compelling part to me. There is literally no way to
> utilize the notes codes for any ref outside of refs/notes currently.
> We don't know if refs/remote-notes is the future, or refs/remotes/
> origin/notes, or something else. But we can't even experiment with it in a
> meaningful way because the plumbing layer is so restrictive.
>
> The notes feature has stagnated. Not many people use it because it  
> requires so much magic to set up and share notes. I think it makes sense to  
> remove a safety feature that is making it harder to experiment with. If the  
> worst case is that people start actually _using_ notes and we get  
> proliferation of places that people are sticking them in the refs hierarchy,
> that is vastly preferable IMHO to the current state, in which few people use
> them and there is little support for sharing them at all.

-Kyle

[1] http://thread.gmane.org/gmane.comp.version-control.git/257281

Kyle J. McKay (1):
  t/t3308-notes-merge.sh: succeed with relaxed notes refs

Scott Chacon (1):
  notes: accept any ref

 notes.c                | 2 +-
 t/t3308-notes-merge.sh | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

-- 
2.1.4

             reply	other threads:[~2015-01-06  8:04 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-06  8:03 Kyle J. McKay [this message]
2015-01-06  8:03 ` [PATCH v2 1/2] notes: accept any ref Kyle J. McKay
2015-01-06  8:03 ` [PATCH v2 2/2] t/t3308-notes-merge.sh: succeed with relaxed notes refs Kyle J. McKay
2015-01-06 10:20   ` Junio C Hamano
2015-01-06 12:27     ` Kyle J. McKay
2015-01-06 18:25       ` Junio C Hamano
2015-01-06 23:29         ` Kyle J. McKay
2015-01-06 23:54           ` Junio C Hamano
2015-01-07  1:27             ` Kyle J. McKay
2015-01-07  0:28           ` Johan Herland
2015-01-07  1:51             ` Kyle J. McKay
2015-01-07 16:14             ` Junio C Hamano
2015-01-07  1:19       ` Johan Herland
2015-01-07  1:19     ` Jeff King
2015-01-07 16:03       ` Junio C Hamano
2015-01-08 10:31         ` Jeff King

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

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

  git send-email \
    --in-reply-to=d4509363c8f670483dacdd2a5070f5a@74d39fa044aa309eaea14b9f57fe79c \
    --to=mackyle@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johan@herland.net \
    --cc=peff@peff.net \
    --cc=schacon@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.
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 NNTP newsgroup(s).