On Wed, Aug 03, 2016 at 08:12:02PM +0100, Richard Ipsum wrote: > On Thu, Jul 28, 2016 at 11:40:55PM -0700, Josh Triplett wrote: > > I'd welcome any feedback, whether on the interface and workflow, the > > internals and collaboration, ideas on presenting diffs of patch series, > > or anything else. > > One other nice thing I've noticed about this tool is the > way series behave like regular git branches: I specify the name > of the series and from then on all other commands act on that > series until told otherwise. Thanks; I spent a while thinking about that part of the workflow. I save the current series as a symbolic ref SHEAD, and everything operates on SHEAD. (I should probably add support for running things like "git series log" or "git series format" on a different series, because right now "until told otherwise" doesn't include a way to tell it otherwise.) One fun detail that took a couple of iterations to get right: I keep separate "staged" and "working" versions per-series, so even with outstanding changes to the cover letter, base, or series, you can always detach or checkout another series without losing anything. If you switch back, all your staged and unstaged changes will remain staged and unstaged where you left them. That solves the "checkout a different series with modifications to the current series" case. > git-appraise looks as though it might also have this behaviour. > I think it's a nice way to do it, since you don't generally > perform more than one review simultaneously. So I may well > use this idea in git-candidate if it's okay. :) By all means. For a review tool like git-candidate, it seems like you'd want even more contextual information, to make it easier to specify things like "comment on file F line L". For instance, what if you spawned the diff to review in an editor, with plenty of extra context and a file extension that'll cause most editors to recognize it as a patch (and specifically a git-candidate patch to allow specialized editor modes), and told people to add their comments after the line they applied to? When the editor exits successfully, you can scan the file, detect the added lines, and save those as comments. You could figure out the appropriate line by looking for the diff hunk headers and counting line numbers. If you use a format-patch diff that includes the headers and commit message, you could also support commenting on those in the same way. Does the notedb format support commenting on those? > I haven't found time to use the tool to do any serious review > yet, but I'll try and post some more feedback when I do. Thanks! - Josh Triplett -- 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