Subject: Re: [PATCH 4/8] git-repack --max-pack-size: add fixup_header_footer()

From: "Shawn O. Pearce" <[email-masked]>
Date: Tue, 1 May 2007 01:06:33 -0400

Dana How <[email-masked]> wrote:
> Add our own version of the one in fast-import.c here.
> Needed later to correct bad object count in header for split pack.
...
> diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
> index bc45ca6..98066bf 100644
> --- a/builtin-pack-objects.c
> +++ b/builtin-pack-objects.c
> @@ -562,6 +562,42 @@ static off_t write_one(struct sha1file *f,
>  	return offset + size;
>  }
>  
> +static void fixup_header_footer(int pack_fd, unsigned char *pack_file_sha1,
> +				char *pack_name, uint32_t object_count)
> +{

This looks a *lot* like the code in fast-import.c.  Why not
refactor both to use the same implementation and stuff it away in
say pack-check.c (for lack of a better place), or start a new file
(pack-write.c)?

There is a *lot* of code in fast-import.c (over 2,000 lines) that
was half-copied from other core code, and that was half created
on its own.  This is also true in index-pack.c and pack-objects.c;
I'd like to see these implementations unify more rather than copy
code from each other.

I know git-blame will identify the original author quite well,
but I'd really like to avoid adding lots more code to maintain
if we can avoid it.

-- 
Shawn.