Re: [PATCH v2] blame: avoid checking if a file exists on the working tree if a revision is provided

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

 



On Mon, Nov 16, 2015 at 11:11 PM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
>
> This subject is a bit long; try to keep it to about 72 characters or less.
>
> More importantly, though, it doesn't give us a high level overview of
> the purpose of the patch, which is that it is fixing blame to work on
> a file at a given revision even if the file no longer exists in the
> worktree.

Sure!

> git-blame documentation does not advertise "blame <file> <rev>" as a
> valid invocation. It does advertise "blame <rev> -- <file>", and this
> case already works correctly even when <file> does not exist in the
> worktree.
>
> git-annotate documentation, on the other hand, does advertise
> "annotate <file> <rev>", and it fails to work when <file> is absent
> from the worktree, so perhaps you want to sell this patch as fixing
> git-annotate instead?

So, do I forget about the blame patch (given that I'm not fixing an
advertised syntax, even if it's supported) and fix annotate instead or
do I fix both? And if I should swing for both, do I create a single
patch or a chain of two patches, one for each builtin?

> This example is certainly illustrative, but an even more common case
> may be trying to blame/annotate a file which existed in an older
> revision but doesn't exist anymore at HEAD, thus is absent from the
> worktree. Such a case could likely be described in a sentence or two
> without resorting to verbose examples (though, not a big deal if you
> keep the example).

K.

>
> A new test or two would be welcome, possibly in t/annotate-tests.sh if
> you consider this also fixing git-blame, or in t8001-annotate.sh if
> you're selling it only as a fix for git-annotate.

I guess I'll wait for feedback about my first question before I decide
what I will do about the tests.

Thank you very much for your comments, Eric.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[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]