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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160804224058.po43kl7w26ockfie@x>
Date:	Thu, 4 Aug 2016 12:40:58 -1000
From:	Josh Triplett <josh@...htriplett.org>
To:	Richard Ipsum <richard.ipsum@...ethink.co.uk>
Cc:	git@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [ANNOUNCE] git-series: track changes to a patch series over time

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