Do we really want git-notes?

[oxij]

Pros:

• Apparently simple.

Kinda pros:

• git notes merge -s union does the union merge that is proposed for the possible future version of the spec. But that merge strategy is easy to do for our use case with sort | uniq anyway.

Cons:

• git notes append adds newlines that have to be filtered out (git-signatures filters in the client, but we can also filter out after append with git notes edit).
• Satisfying this spec (ordering) seems a bit hard when having conflicts.
• Doesn't allow stage-commit workflow I would like to have.
• It doesn't make much sense to call those "notes", really, as you can't read those notes in git-log without decoding them with git-wotr first anyway.

I think we want the same approach, but implemented via git builtins.

[lrvick]

Notes is just the only method git offers to attach arbitrary metadata to a commit.

If we push for upstream git inclusion we should seek a first class object for this use case, imo.

[Ekleog]

Suggestion (inspired from how I understand git-notes to work):

• Everything goes in a refs/reviews branch
• On this branch:
  • File [oid] contains the sorted list of reviews for commit [oid]
  • File [patch-id] contains the sorted list of reviews for patch [patch-id] (depending on the outcome of Multi-level anchoring modes #3, which I haven't read yet, so potentially tree-id too)

The advantages of this scheme are that:

• For commit-id's, it's compatible with git-notes (it's the exact same format as git-notes would store)
• For tree-id's or patch-id's, it's a logical extension

The drawbacks (shared with git-notes) are that:

• The files aren't namespaced by the first two characters of their hash, so this may lead to big directories (though I'm not sure that's actually an issue because iirc git doesn't store directories as directories anyway, so they would never hit the file system this way)
• Merging is a PITA, we should make sure tooling thinks about how and when to merge review branches. I'm not really sure this should be discussed in the spec, though, but it can impact whether we want to allow out-of-order reviews in these files or whether we want to allow merge commits on the review branch.

[oxij]

I would also put them under different directories in the branch so that you won't mix different id schemes. E.g. oid/<commit-id> and patch/<patch-id>.

[Ekleog]

Hmm… The drawback of this approach is that it's no longer backwards-compatible with git-notes. As there should be no collision between patch-id hashes and commit-id hashes (unless something goes really weird), is there an advantage I don't see to doing this? :)

[oxij]

But why would you want it to be compatible with git-notes if they are not really human-readable anyway?

[Ekleog]

This can help make a bash implementation easier to write.

[lrvick]

Hmm. If I am parsing this correctly it means someone will need to tap their yubikey multiple times if you want to dual sign the patch-id and the commit-id and yet again if you want to sign some kind of metadata ID.

I have been thinking a lot about this and I now am leaning more and more to the KISS ideas we have had. Also because I need to ship git-signatures for multiple orgs soon.

Why not "refs/notes/signatures" simply hold a dirt simple delimited format of:

<commit-id(required)>:<patch-id(optional)>:<child-id(optional)>

child-id could contain any arbitrary git object. Could be another git-note, or to a ref of a whole folder of files that different tools can care about as they prefer.

Any added metadata could be pushed to this child-id object and we can allow each tool to define their own interpretations of whatever in that object, but it won't be part of the v1 spec.

This leaves us entirely free to bike-shed on multipart formats or individual file formats after we get this MVP spec shipped that can solve most classes of problems today with an open path for tool-specific extensions... and we can see how those extensions play out in pratice.

[Ekleog]

@lrvick IMO the need for a MVP shouldn't block good speccing. Have you considered #6 (comment) for shipping your MVP while speccing is underway?

Also, your proposal doesn't handle the question of how to associate data from an id to a list of signatures.

And for the problem of having to tap twice, I think it's handled in the discussion starting at #3 (comment), so I suggest to continue that discussion there :) (tl;dr of it: all the information one can want is already in the commit-id, so patch-id signature is really commit-id signature with a different verification scheme that ignores parts of the commit)

[lrvick]

Thanks for catching me up. Juggling a bunch of projects at work atm and not been keeping up with this as much as I would like.

Will follow up on those.

[Ekleog]

Great! So I think currently this particular issue is blocked on #3, as depending on whether there's a need for storing signatures to things other than commit-id (which should be discussed in #3) we may or may not want to store notes in git-notes format :)