git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Josh Triplett <josh@joshtriplett.org>
To: Richard Ipsum <richard.ipsum@codethink.co.uk>
Cc: git@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [ANNOUNCE] git-series: track changes to a patch series over time
Date: Thu, 4 Aug 2016 12:40:58 -1000	[thread overview]
Message-ID: <20160804224058.po43kl7w26ockfie@x> (raw)
In-Reply-To: <20160803191202.GA22881@salo>

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

  reply	other threads:[~2016-08-04 22:41 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-29  6:40 [ANNOUNCE] git-series: track changes to a patch series over time Josh Triplett
2016-07-29 10:10 ` Richard Ipsum
2016-07-29 11:04   ` Josh Triplett
2016-07-29 12:44     ` Richard Ipsum
2016-07-29 13:00       ` Josh Triplett
2016-07-31 14:35         ` Richard Ipsum
2016-07-29 16:59       ` Stefan Beller
2016-07-31 14:09         ` Richard Ipsum
2016-08-01  5:04   ` Christian Couder
2016-08-01  7:55     ` Eric Wong
2016-08-01  8:59       ` Josh Triplett
2016-08-01  9:53         ` Richard Ipsum
2016-08-01 21:19         ` Eric Wong
2016-08-01 15:14 ` Stephen Warren
2016-08-01 18:37   ` Josh Triplett
2016-08-15 18:17     ` Simon Glass
2016-08-15 20:05       ` Josh Triplett
2016-08-03 19:12 ` Richard Ipsum
2016-08-04 22:40   ` Josh Triplett [this message]
2016-08-10  9:37     ` Richard Ipsum
2016-08-10 22:07       ` Josh Triplett
2016-08-11  6:23         ` Eric Wong
2016-08-24 10:56         ` Jakub Narębski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160804224058.po43kl7w26ockfie@x \
    --to=josh@joshtriplett.org \
    --cc=git@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=richard.ipsum@codethink.co.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).