Re: [PATCH v2 2/3] blame,cat-file: Demonstrate --textconv is wrongly running converter on symlinks

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

 



On Tue, Sep 21, 2010 at 10:18:11PM +0200, Matthieu Moy wrote:
> Kirill Smelkov <kirr@xxxxxxxxxxxxxxxxxxx> writes:
> 
> > I know about --- and that content after it and up to patch itself goes
> > to /dev/null. The text here was intended to stay in the commit message,
> > and ~~~~ served as a separator in that message (git commit hook merges
> > several blank lines into one, so one can't separate text parts with
> > several empty lines, that's why I used this separator).
> >
> > If it's ugly, let's omit it - I don't insist, but i don't understand why
> > 'NOTE:' looks odd?
> 
> That was especially about the combination of both, but I don't really
> care. Consider my remarks as food for thoughts, not real objections.

Ok

> > This note clearly says "git diff is not affected, that's why we don't
> > write new tests for it".
> 
> I disagree with the implication. Git diff is not affected because it
> was done right, but behaving correctly doesn't mean you don't need
> tests. Checking the behavior of diff with tests wouldn't harm (but
> that's not serious not to do it).

Yes, my implication was not correct, and I agree tests woudn't harm. But
as Jeff already wrote in another mail, he'll test-cover the diff case,
so I'll let myself to be lazy here (thanks Jeff!) :)

> >> Reviewed-by: Matthieu Moy <Matthieu.Moy@xxxxxxxxxxxxxxx>
> >
> > Thanks :)
> >
> > Is it for this one patch, or does it apply to the whole series?
> 
> To the serie, yes.

Thanks. Will repost v3 soon. Hopefuly it will be ok to merge it then.

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