Jacob Keller <jacob.keller@xxxxxxxxx> writes: > The other issue here is that expand_notes_ref is called on the --ref > argument waaay before the current code even decides if the operation > is "read" or "write". Thus we'd have to break this out and handle > things very differently. I think you hit the right nail here. The handling of --ref argument is what you need to adjust to the new world order. And "safety" is a red herring. Those who are used to run git log --notes=refs/heads/master and relies on it to refer to refs/notes/refs/heads/master must continue to be able to do so. Changing expand_notes_ref() the way the proposed patch does will break them, won't it? "safety" is not the only (or even the primary) requirement, but the correctness must also be kept. > It seems like a lot more heavy lifting to change the entire flow of > how we decide when to expand "--ref" for "read" operations vs "write" > operations. > > That is, git notes list. > > It's easy to change behavior of git notes merge as it handles its > argument for where to merge from separately, but it's not so easy to > change git notes show... Yes, exactly. I am _more than_ OK to see that read-only accesses to the notes tree allowed anything under refs/ (like the proposed patch did) and also a raw 40-hex object name in the endgame, but I really would not want to see "we meant well and attempted to enhance 'notes merge' but we ended up regressing the behaviour of unrelated operations all of a sudden". If you cannot do your change without breaking the existing users, then you at least need a sound transition plan. But I am not sure yet if you have to break the world (yet). Let me think aloud a bit more. There aren't all that many callers of expand_notes_ref(). My preference is to leave that function as-is, especially keep the behaviour of the caller in revision.c that handles --show-notes= (and --notes= that is its synonym) the same as before. All the other callers are only reachable from the codepath that originates from cmd_notes(), if I am reading the code correctly, and that can be enhanced without breaking the existing users. One obvious way to do so would be to make "--ref" to keep the call to expand_notes_ref(), and add another "--notes-rawref" or whatever option that does not restrict it to "refs/notes", but there may be other ways. -- 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