Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1460040 > unrolled thread

Re: [ANNOUNCE] git-series: track changes to a patch series over time

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [ANNOUNCE] git-series: track changes to a patch series over time Josh Triplett <josh@joshtriplett.org> - 2016-08-11 00:10 +0200
    Re: [ANNOUNCE] git-series: track changes to a patch series over time Eric Wong <e@80x24.org> - 2016-08-11 08:30 +0200
    Re: [ANNOUNCE] git-series: track changes to a patch series over time Jakub Narębski <jnareb@gmail.com> - 2016-08-24 14:00 +0200

#1460040 — 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:
>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.

>> > 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.

>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.

>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.

>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).

>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).

[toc] | [next] | [standalone]

#1460195

Josh Triplett <josh@joshtriplett.org> wrote:
> On August 9, 2016 11:37:31 PM HST, Richard Ipsum <richard.ipsum@codethink.co.uk> wrote:
> 
> >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.

Thanks, I'd like to see an LKML version, too :)  First, I want
to ensure public-inbox can handle large repos better, first.
public-inbox.org/git has been doing well so far, even on a
low-end VM with 2 cores and 2GB RAM.

I don't have anything close to full history of LKML, and
download.gmane.org is down, right now :<  I'd use NNTP, but
news.gmane.org gets overloaded from slrnpull and I even got
temporarily banned there before discovering download.gmane
:x

Maybe I'll do what was done with linux.git in 2005 and just
ignore old mail for a while...

[toc] | [prev] | [next] | [standalone]

#1469366

W dniu 11.08.2016 o 00:07, Josh Triplett pisze:
> On August 9, 2016 11:37:31 PM HST, Richard Ipsum
> <richard.ipsum@codethink.co.uk> wrote:
>> On Thu, Aug 04, 2016 at 12:40:58PM -1000, Josh Triplett wrote:
[...]

>>> 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.

There is also cover letter and interdiff, and one would want to
be able to comment also on those.

So how notedb solve problem of in-diff comments, in-commit comments,
post-commit comments, whole series cover letter and cover-letter
comments, interdiff and interdiff message / comments?


Nb. GitHub Pull Requests include only some of those, compared to
the mailing list / Usenet news interface.
 
>> 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...)

I wonder if we could use 'parent' header of a commit message for this,
or equivalent...
 
>> 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.

...which is as easy to set up and use as mailing list, for sending
patches, applying patches, and patch review.  And/or provide 
bi-directional interface to the mailing list (I think Debian 
infrastructure tries to be (inter)operable by email).

There are various federated technologies (like pump.io), the
problem might be their popularity.

>> It may be worth reflecting that one reason some organizations 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. Of course 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.

I think there are command-line tools that allow to submit comments
to GitHub.

-- 
Jakub Narębski

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web