Re: [ANNOUNCE] git-series: track changes to a patch series over time
On August 9, 2016 11:37:31 PM HST, Richard Ipsum <richard.ipsum@codethink.co.uk> wrote:
Show 28 quoted lines
>On Thu, Aug 04, 2016 at 12:40:58PM -1000, Josh Triplett wrote:
>> 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.)
>
>Apologies for this delayed response,
>I needed time to gather my thoughts,
>and also to fix the perl libgit2 binding to allow me to use
>your symbolic ref suggestion. :p
Yeah, during git-series development I ended up doing some work on both libgit2 and git2-rs. :)
>Though it turns out that libgit2 doesn't currently allow
>me to write arbitrary data to a symbolic ref as git-symbolic-ref(1)
>will,
>so this still needs to be fixed somehow.
What arbitrary data do you need to write?
Also, note that you want to put your symbolic ref in refs/, not directly in .git, so that git takes it into account for object reachability.
Show 33 quoted lines
>> > 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.
>
>I really like this idea, the current interface for commenting is a
>little
>tedious I find.
>
>>
>> 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?
>
>Comments in notedb are just a git note keyed on the sha of the
>commit being commented on, I'm not certain what advantage a
>format-patch
>diff provides in this case?
I meant for opening in an editor to write email-reply-style comments. The review tool and review storage format should allow commenting on commit messages, not just diffs.
Show 5 quoted lines
>I've been closely following the 'patch submission process' thread,
>and given the discussion there I'm having doubts over the value
>of comments in git-candidate vs the mailing list. It seems to me that
>git-candidate has many of the disadvantages of Github/Gitlab when it
>comes to comments, for example, there is no threading.
That's not inherent, though. You could allow commenting on a comment easily enough. (Of course, at some point you've recreated email-style in-reply-to headers...)
>Also the system would be less open than the mailing list, since,
>as it stands currently you would require push access to the repository
>to comment on anything.
You'd need a federation mechanism.
Show 10 quoted lines
>It may be worth reflecting that one reason some organisations
>have switched away from mailing list reviews to Github/Gitlab is that
>they provide patch tracking, where the mailing list provides none,
>so patches there can be 'lost'. So instead of trying to reimplement
>an entire Gerrit/Github/Gitlab ui on the commandline, I wonder whether
>it would be sufficient to add the minimum functionality necessary
>to provide git with native patch tracking, and leave comments for the
>mailing list. Ofcourse this is exactly what git-series seems to do,
>so in some sense I may be advocating dropping my own work in favour of
>improving git-series.
I think the two serve different (though related) functions. I'd love to be able to use a text editor and command-line tool to produce and submit comments to systems like Gerrit or GitHub.
Show 6 quoted lines
>On the other hand, relying on the mailing list means that some of the
>history of a series is left outside of the repository which is
>anathema to the goal of git based/stored review, not least because
>mail archives are centralised.
>(which can obviously be problematic (as we've seen recently with
>gmane))
Agreed. You can always choose to *intentionally* discard history, or store it elsewhere, but having it in the repository allows you to make that decision with all the data really available (and easily backed up).
Show 5 quoted lines
>Maybe there's a better solution to this problem than git-candidate
>then,
>maybe we can just invent some wonderful new subcommand that fetches
>a mailing list archive into a git repo, for those that want that,
>I don't know.
public-inbox seems to address that use case. I'd love to see a public-inbox version of LKML, with full history. I don't think that fully solves the review storage and interchange problem, but it seems like an *excellent* solution for email archiving, and for distribution of archives.
>Out of interest, did you have any thoughts on Notedb itself with
>respect
>to its suitability for git-series?
Seems like a potentially reasonable format for storing reviews. I think the two could work well together, with git-series storing all the historical versions of a series, and then a notedb could reference those commits.
I've given some thought to using git-series as a server-side storage format for something like a pull request. I think it might make sense for a tool like Gerrit or GitLab to allow pushing and pulling series branches (that must fast-forward) to a special ref (like Gerrit's refs/for/master).
Josh Triplett <josh@joshtriplett.org>