From: Martin Langhoff <martin.langhoff@gmail.com>
Date: Tue, 23 May 2006 08:13:10 GMT
Subject: Re: [PATCH 2/2] cvsimport: cleanup commit function
Message-ID: <46a038f90605230113x2f6b0e4bq5a2ea97308b495e0@mail.gmail.com>
In-Reply-To: <20060523070007.GC6180@coredump.intra.peff.net>

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?

> +sub update_index (\@\@) {
> +       my $old = shift;
> +       my $new = shift;

Would it not make more sense to just pass them as plain parameters?

> +       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.

cheers,


martin