Re: [PATCH] filter-branch: return 2 when nothing to rewrite

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jeff King <peff@xxxxxxxx> writes:

> On Thu, Mar 15, 2018 at 02:03:59PM +0100, Michele Locati wrote:
>
>> Using the --state-branch option allows us to perform incremental filtering.
>> This may lead to having nothing to rewrite in subsequent filtering, so we need
>> a way to recognize this case.
>> So, let's exit with 2 instead of 1 when this "error" occurs.
>
> That sounds like a good feature. It doesn't look like we use "2" for
> anything else currently.

I do not want to sound overly negative against the first
contribution from a new contributor, but I am not sure if this is a
good idea.  While I do agree that the caller of filter-branch would
want _some_ way to tell if the call

 - got some new stuff,
 - got no error but did not get anything new, or
 - failed

and act accordingly, changing the exit code to a non-zero value for
the second case above would mean that existing scripts that have
happily been working would suddenly start failing.  Due to the lack
of an easy way to tell the first two cases apart, they may have been
doing _extra_ work after calling filter-branch when it found no new
development (resulting in an expensive no-op), or perhaps they
implemented their own way to tell the second case apart from the
first one and efficiently omitting extra work in the second case
already.  In either case, these scripts will get broken with this
change.

So I'd respond with a mild "no" with "can't we allow callers to tell
the first two cases apart in some other way so that we do not have
to break existing scripts?".

>> ---
>>  git-filter-branch.sh | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> This should probably get a mention in the manpage at
> Documentation/git-filter-branch.txt, too.

Whatever solution we eventually end up with, it needs to be
documented.

Thanks.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux