{"thread":{"id":"27127","subject":"Git notes list/show ","startedAt":"2011-04-18T18:19:59Z","lastAt":"2011-04-20T07:35:17Z","messageCount":6,"participants":["Vincent van Ravesteijn","Jeff King","Michael J Gruber"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"166040","messageId":"4DAC80CF.8020704@lyx.org","threadId":"27127","inReplyTo":null,"subject":"Git notes list/show ","fromName":"Vincent van Ravesteijn","fromEmail":"vfr@lyx.org","sentAt":"2011-04-18T18:19:59Z","receivedAt":"2011-04-18T18:19:59Z","isPatch":false,"sender":{"key":"vfr@lyx.org","avatar":"https://avatars.githubusercontent.com/u/687868?v=4"},"body":"Hi all,\n\nConsidering a work-flow using topic branches just as the git development uses itself, I do miss a certain feature.\n\nWhen a feature/patch is proposed, it will \"cook\" for a while in \"pu\" and \"next\". During this period people can comment on the patch. These comments can be stored in git notes added to the commits on which is commented.\n\nAt a certain point I want to decide whether the feature can be merged into \"next\" or \"master\", so I want to collect all notes for a certain branch to see what still has to be done before this feature can be merged. One can think about this of automatically generating a \"What's cooking in X\" listing\nthe topic branches and the comments for the commits in each branch.\n\nThat's why I want to (try to) implement the feature that 'git notes list' and 'git notes show' accept a revision range and collect all notes for the commits in this range.\n\nDo you think this will be a useful feature and that it has chances to be merged into git ?\n\nVincent\n"},{"id":"166041","messageId":"20110418182724.GB11250@sigill.intra.peff.net","threadId":"27127","inReplyTo":"4DAC80CF.8020704@lyx.org","subject":"Re: Git notes list/show ","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-04-18T18:27:24Z","receivedAt":"2011-04-18T18:27:24Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Apr 18, 2011 at 08:19:59PM +0200, Vincent van Ravesteijn wrote:\n\n> When a feature/patch is proposed, it will \"cook\" for a while in \"pu\"\n> and \"next\". During this period people can comment on the patch. These\n> comments can be stored in git notes added to the commits on which is\n> commented.\n\nIf you haven't already seen it, you might find the \"notes/*\" refs here:\n\n git://repo.or.cz/git/trast.git\n\nThey reference the messages discussing the patch on the mailing list by\nmessage-id and gmane link.\n\n> That's why I want to (try to) implement the feature that 'git notes\n> list' and 'git notes show' accept a revision range and collect all\n> notes for the commits in this range.\n\nYou can do more-or-less what you want with something like:\n\n git log --format=\"%h %s%n%N\"\n"},{"id":"166064","messageId":"4DAD371F.9040003@drmicha.warpmail.net","threadId":"27127","inReplyTo":"20110418182724.GB11250@sigill.intra.peff.net","subject":"Re: Git notes list/show ","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-04-19T07:17:51Z","receivedAt":"2011-04-19T07:17:51Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jeff King venit, vidit, dixit 18.04.2011 20:27:\n> On Mon, Apr 18, 2011 at 08:19:59PM +0200, Vincent van Ravesteijn wrote:\n> \n>> When a feature/patch is proposed, it will \"cook\" for a while in \"pu\"\n>> and \"next\". During this period people can comment on the patch. These\n>> comments can be stored in git notes added to the commits on which is\n>> commented.\n> \n> If you haven't already seen it, you might find the \"notes/*\" refs here:\n> \n> git://repo.or.cz/git/trast.git\n> \n> They reference the messages discussing the patch on the mailing list by\n> message-id and gmane link.\n> \n>> That's why I want to (try to) implement the feature that 'git notes\n>> list' and 'git notes show' accept a revision range and collect all\n>> notes for the commits in this range.\n> \n> You can do more-or-less what you want with something like:\n> \n> git log --format=\"%h %s%n%N\"\n\nExactly, the log family does that and more, since it makes the whole\nrevision walk machinery available.\n\nBut since the OP is volunteering to code for notes :-)\nWe could need a feature which allows to log the history of a note. The alias\n\n`git noteslog' is aliased to `!sh -c 'git log $(git notes get-ref) \"$@\"' -'\n\ngives you the history of the notes tree (try it with \"-p\"), but\nsometimes I would like the history of the notes to a specific commit, and in\n\ngit noteslog -p -- $(commit)\n\nI would have to use for $(commit) all possible breakdowns of the sha1 of\nthe commit for all possible notes tree structures. It feels as of the\nrevision walker needs to learn another pathspec, say\n\n\":(note):\"\n\nin line with our magic pathspec discussion.\n\nMichael\n"},{"id":"166102","messageId":"20110419203211.GA12071@sigill.intra.peff.net","threadId":"27127","inReplyTo":"4DAD371F.9040003@drmicha.warpmail.net","subject":"Re: Git notes list/show ","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-04-19T20:32:11Z","receivedAt":"2011-04-19T20:32:11Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 19, 2011 at 09:17:51AM +0200, Michael J Gruber wrote:\n\n> But since the OP is volunteering to code for notes :-)\n> We could need a feature which allows to log the history of a note. The alias\n> \n> `git noteslog' is aliased to `!sh -c 'git log $(git notes get-ref) \"$@\"' -'\n> \n> gives you the history of the notes tree (try it with \"-p\"),\n\nHmm, I just use \"git log notes/\", which works fine. It does\nhelp if you know that the default ref is \"notes/commits\", though.\n\nIt's not something I do often, though (most of my notes use has been\nthings that automatically make notes, so the history tends to be\nuninteresting and useful only for debugging the note-making code).\n\n> sometimes I would like the history of the notes to a specific commit, and in\n> \n> git noteslog -p -- $(commit)\n> \n> I would have to use for $(commit) all possible breakdowns of the sha1 of\n> the commit for all possible notes tree structures. It feels as of the\n> revision walker needs to learn another pathspec, say\n> \n> \":(note):\"\n> \n> in line with our magic pathspec discussion.\n\nThat's a clever solution. It is a little non-intuitive for a user to\nneed to know about notes storage, though. Maybe you were already\nthinking this, but we could have something like:\n\n git notes log [revs] [--] [pathspec]\n\nwhere \"[revs]\" are checked for in refs/notes/*, defaulting to\n\"refs/notes/commits\". And each element of the pathspec gets the\n\":(note):\" magic automatically. I wonder if we could even resolve the\npathspec bits as regular refs.\n\nSo you could write:\n\n # long form, just as you can do with \"git log\"\n git notes log notes/commits -- \":(note):`git rev-parse HEAD`\"\n\n # or with automagic ref lookup for pathspec\n git notes log notes/commits -- HEAD\n\n # and automagic default ref\n git notes log -- HEAD\n\n # and I think you should be able to write a disambiguator similar to\n # what we use for the revs/paths distinction, but this time for\n # notes-refs versus regular refs. And then drop the \"--\":\n git notes log HEAD\n\nI think it would need a little refactoring of setup_revisions() to be\nmore flexible, but most of the hard work is already done by the usual\nrevision traversal mechanism.\n\n-Peff\n"},{"id":"166131","messageId":"4DAE8867.8010704@drmicha.warpmail.net","threadId":"27127","inReplyTo":"20110419203211.GA12071@sigill.intra.peff.net","subject":"Re: Git notes list/show ","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-04-20T07:16:55Z","receivedAt":"2011-04-20T07:16:55Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jeff King venit, vidit, dixit 19.04.2011 22:32:\n> On Tue, Apr 19, 2011 at 09:17:51AM +0200, Michael J Gruber wrote:\n> \n>> But since the OP is volunteering to code for notes :-)\n>> We could need a feature which allows to log the history of a note. The alias\n>>\n>> `git noteslog' is aliased to `!sh -c 'git log $(git notes get-ref) \"$@\"' -'\n>>\n>> gives you the history of the notes tree (try it with \"-p\"),\n> \n> Hmm, I just use \"git log notes/\", which works fine. It does\n> help if you know that the default ref is \"notes/commits\", though.\n\nSure. That's what the alias does.\n\n> It's not something I do often, though (most of my notes use has been\n> things that automatically make notes, so the history tends to be\n> uninteresting and useful only for debugging the note-making code).\n\nIn particular, the log messages \"Notes added by...\" are useless. I use\nnotes for patch boiler plates and branch annotations, and in these cases\nhistory can be interesting (between versions of a patch, e.g.).\n\n>> sometimes I would like the history of the notes to a specific commit, and in\n>>\n>> git noteslog -p -- $(commit)\n>>\n>> I would have to use for $(commit) all possible breakdowns of the sha1 of\n>> the commit for all possible notes tree structures. It feels as of the\n>> revision walker needs to learn another pathspec, say\n>>\n>> \":(note):\"\n>>\n>> in line with our magic pathspec discussion.\n> \n> That's a clever solution. It is a little non-intuitive for a user to\n> need to know about notes storage, though. Maybe you were already\n> thinking this, but we could have something like:\n> \n> git notes log [revs] [--] [pathspec]\n> \n> where \"[revs]\" are checked for in refs/notes/*, defaulting to\n> \"refs/notes/commits\". And each element of the pathspec gets the\n> \":(note):\" magic automatically. I wonder if we could even resolve the\n\nHere we are back at my alias is a first step in that direction... \"git\nnotes log\" should do all that, of course.\n\n> pathspec bits as regular refs.\n> \n> So you could write:\n> \n> # long form, just as you can do with \"git log\"\n> git notes log notes/commits -- \":(note):`git rev-parse HEAD`\"\n\nI don't think \"notes log\" should take a notes ref argument like that. It\nalready has \"--ref\" for that purpose, so I would suggest\n\ngit notes [--ref ] log []\n\nwith being mapped to :(notes): automatically, \ndefaulting to HEAD. That is much more in line with the other notes\nsubcommands. (We may or may note check for \":(notes):\" being there already.)\n\n> \n> # or with automagic ref lookup for pathspec\n> git notes log notes/commits -- HEAD\n> \n> # and automagic default ref\n> git notes log -- HEAD\n> \n> # and I think you should be able to write a disambiguator similar to\n> # what we use for the revs/paths distinction, but this time for\n> # notes-refs versus regular refs. And then drop the \"--\":\n> git notes log HEAD\n> \n> I think it would need a little refactoring of setup_revisions() to be\n> more flexible, but most of the hard work is already done by the usual\n> revision traversal mechanism.\n\n? I haven't checked in detail, but I think all we need is:\n\n- Make \"git notes --ref log \" call \"git log\n -- :(notes):\" (and pass on log options).\n\n- Make the pathspec \":(notes):\" resolve to and match\nwith all possible breakdowns of which we accept for a notes tree.\n\nMichael\n"},{"id":"166133","messageId":"20110420073517.GB1971@sigill.intra.peff.net","threadId":"27127","inReplyTo":"4DAE8867.8010704@drmicha.warpmail.net","subject":"Re: Git notes list/show ","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-04-20T07:35:17Z","receivedAt":"2011-04-20T07:35:17Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Apr 20, 2011 at 09:16:55AM +0200, Michael J Gruber wrote:\n\n> > So you could write:\n> > \n> > # long form, just as you can do with \"git log\"\n> > git notes log notes/commits -- \":(note):`git rev-parse HEAD`\"\n> \n> I don't think \"notes log\" should take a notes ref argument like that. It\n> already has \"--ref\" for that purpose, so I would suggest\n> \n> git notes [--ref ] log []\n\nHmm. You are probably right. I was modeling it after \"log\", but it is a\nnotes subcommand. I was thinking you could also do something like:\n\n git notes log commits some-other-notes $sha1\n\nor even\n\n # see what any notes ref has to say about this commit\n git notes log --all $sha1\n\nbut it is probably a better idea to keep the interface more consistent\nwith the other notes subcommands.\n\n> git notes [--ref ] log []\n>\n> with being mapped to :(notes): automatically, \n> defaulting to HEAD. That is much more in line with the other notes\n> subcommands. (We may or may note check for \":(notes):\" being there already.)\n\nI think it should be more like:\n\n git notes [--ref ] [ ...]\n\nThat is:\n\n 1. There may be arbitrary rev options. Because you might want \"log\n -p\", or \"log --format\".\n\n 2. We may want to allow multiple revs (e.g., if you have a patch\n series with 5 commits, you may want to see the history of any notes\n that refer to any of those commits).\n\nI think (1) means we may need to feed the result to the revision\noptions-parser.\n\n> > I think it would need a little refactoring of setup_revisions() to be\n> > more flexible, but most of the hard work is already done by the usual\n> > revision traversal mechanism.\n> \n> ? I haven't checked in detail, but I think all we need is:\n> \n> - Make \"git notes --ref log \" call \"git log\n> -- :(notes):\" (and pass on log options).\n\nI was worried about how to get the log options. If you see:\n\n git notes log --foo bar\n\nis \"bar\" a revision, or an argument to --foo? I think right now the\nanswer is always \"revision\", but that is only because the revision\noptions do not currently use parse-options, which would allow\n\"--format=foo\" to be written as \"--format foo\". So I'd rather not rely\non that.\n\n> - Make the pathspec \":(notes):\" resolve to and match\n> with all possible breakdowns of which we accept for a notes tree.\n\nI was assuming that :(notes): would only accept a sha1. But it probably\nmakes sense for it to handle resolution itself.\n\n-Peff\n"}]}