Re: [PATCH/v2] git-basis, a script to manage bases for git-bundle

From: Jeff King <peff@peff.net>
Date: Jul 2, 2008, 03:21 UTC
Message-ID: <20080702032155.GA13581@sigill.intra.peff.net>
In-Reply-To: <c376da900807011836i76363d74n7f1b87d66ba34cd6@mail.gmail.com>

On Tue, Jul 01, 2008 at 09:36:20PM -0400, Adam Brewster wrote:

> Makes sense,  I thought it was small enough for one commit, but I'll
> split it up when I resubmit.

I think in this instance it is not too big a deal either way. I am just trying to help encourage good habits. :)

> > Style: we usually spell NUL as '\0'.
> 
> Okay.  I can also include a third patch for the code I cut-and-pasted.

Heh. I said "usually", but I guess even Junio makes mistakes (unless I am dreaming such a style directive, but ISTR it being mentioned before on the list). I wouldn't bother with the style cleanup in rev-list (usually for such small things, we just wait until touching that part of the code).

> > Why make a hash when the only thing we ever do with it is "keys %new"?
> > Shouldn't an array suffice?
> 
> It's probably a non-issue, but using a hash will prevent duplicates.

Ah, true. And there will be duplicates here, if you have multiple refs at the same spot in your bundle list. So it should remain as you have it.

Show 5 quoted lines

> If all goes well then you're right, but I thought old objects should
> be kept around  in case the user has some reason to manually delete
> them.  As it is, you can go into the basis file and delete everything
> past a given date line and be back where you were.  If I delete the
> redundant objects, then that's not always possible.

Hmm, and that might be useful. Probably the best thing would be to leave it as-is for now, then, with a note. Then we can decide the best pruning strategy if and when it becomes an issue.

Show 8 quoted lines

> Maybe I'm a idiot, but I can't find any built-in date to string
> functions that do nice things like print the date the way the user
> says he likes to look at dates.
> 
> I updated the comment line to be "# <git-date> // `date`" where
> git-date is as per git-fast-import (seconds since 1969 +/-TZ).  If
> automatic pruning ever happens, the git-date will be used, so `date`
> is just for humans.

That sounds reasonable.

> > Notably absent: any tests.
> Working on those.  I'll also include tests for git-bundle.

Great. Glancing over Junio's comments, though, it might make sense to integrate this more tightly with git-bundle, in which case the perl stuff would go away. So I'll let you work out with him which is the best route.

-Peff