git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Avery Pennarun <apenwarr@gmail.com>
Cc: git@vger.kernel.org, gitster@pobox.com
Subject: Re: [PATCH v2 1/3] filter-branch: add new --blob-filter option.
Date: Fri, 13 Jun 2008 02:25:46 -0400	[thread overview]
Message-ID: <20080613062546.GD26768@sigill.intra.peff.net> (raw)
In-Reply-To: <1213318344-26013-1-git-send-email-apenwarr@gmail.com>

On Thu, Jun 12, 2008 at 08:52:22PM -0400, Avery Pennarun wrote:

> It was easy enough to work up the patch below, which allows
> 
>   git filter-branch --blob-filter 'tr a-z A-Z'

First, two procedural complaints:

  1. We're supposed to be in rc freeze, so this is not a great time to
     publish a new feature. ;)

  2. When bringing back an old patch, please please please give at least
     a little bit of cover letter context. "Here is what happened last
     time, here are the reasons this patch was not accepted before, and
     here is {why I think it that decision was wrong, what I have done
     to improve the patch, etc}.

IIRC, the situation last time had two issues:

  1. it was a one-off "we're not sure if this is really useful" patch

  2. it was unclear whether paths should be available, and if they were,
     there was an issue of encountering the same hash at two different
     paths.

I assume your answer to '1' is "I have been using this and it is
useful". And for '2', it looks like you have extended the cache
mechanism to take into account the sha1 and the path, which I think is
the right solution (and I am pleased to see it looks like the final test
covers the exact situation I was concerned about).

So:

(for 1/3):
Signed-off-by: Jeff King <peff@peff.net>

(for the others (and for 1/3, do I get to ack my own patch?)):
Acked-by: Jeff King <peff@peff.net>

-Peff

  parent reply	other threads:[~2008-06-13  6:27 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-13  0:52 [PATCH v2 1/3] filter-branch: add new --blob-filter option Avery Pennarun
2008-06-13  0:52 ` [PATCH v2 2/3] filter-branch --blob-filter: speed/flexibility improvements Avery Pennarun
2008-06-13  0:52   ` [PATCH v2 3/3] filter-branch --blob-filter: add tests Avery Pennarun
2008-06-13  6:25 ` Jeff King [this message]
2008-06-13 16:10   ` [PATCH v2 1/3] filter-branch: add new --blob-filter option Avery Pennarun

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

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

  git send-email \
    --in-reply-to=20080613062546.GD26768@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=apenwarr@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /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.
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 NNTP newsgroup(s).