Re: [PATCH 2/2] cvsimport: cleanup commit function

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

 



On 5/23/06, Junio C Hamano <junkio@xxxxxxx> wrote:
"Martin Langhoff" <martin.langhoff@xxxxxxxxx> writes:

> Jeff,
>
> good stuff -- aiming at exactly the things that had been nagging me.
> Some minor notes on top of what junio's mentioned...
>
>> +    die "unable to open $f: $!" unless $! == POSIX::ENOENT;
>> +    return undef;
>
> Heh. Is that the return of the living dead?

Note the trailing "unless" there.

Of course. I had actually missed the closing quotes, and thought the
error msg wanted to talk about POSIX. 'twas late in the day, seems
like most of my comments in this email were rather stoopid.

>> +sub update_index (\@\@) {
>> +       my $old = shift;
>> +       my $new = shift;
>
> Would it not make more sense to just pass them as plain parameters?

Meaning...?  Perl5 can pass only one flat array, so the above is
a standard way to pass two arrays.

Meaning I am stupid :(

>> +       print "Committed patch $patchset ($branch $commit_date)\n" if
>
> Given that we have that -- should we remember it and avoid re-reading
> the headref from disk? A %seenheads cache would save us 99.9% of the
> hassle.
>
> In related news, I've dealt with file reads from the socket being
> memorybound. Should merge ok.

Merged OK, and I think your last suggestion makes sense.  I'll
go to bed after pushing out Jeff's two patches and yours.

I'll look into caching headrefs tonight if noone beats me to it.




martin
-
: 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]