All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Shawn O. Pearce" <spearce@spearce.org>
To: Dana How <danahow@gmail.com>
Cc: Junio C Hamano <junkio@cox.net>, Git Mailing List <git@vger.kernel.org>
Subject: Re: [PATCH 4/8] git-repack --max-pack-size: add fixup_header_footer()
Date: Tue, 1 May 2007 01:06:33 -0400	[thread overview]
Message-ID: <20070501050633.GZ5942@spearce.org> (raw)
In-Reply-To: <463679EB.2010301@gmail.com>

Dana How <danahow@gmail.com> 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.

  reply	other threads:[~2007-05-01  5:06 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-04-30 23:21 [PATCH 4/8] git-repack --max-pack-size: add fixup_header_footer() Dana How
2007-05-01  5:06 ` Shawn O. Pearce [this message]
2007-05-01  5:41   ` Dana How
2007-05-01  6:03     ` Shawn O. Pearce
2007-05-01  7:32       ` Johannes Schindelin
2007-05-01 17:48       ` Nicolas Pitre
2007-05-01 17:58         ` Dana How
2007-05-01 18:39           ` Nicolas Pitre
  -- strict thread matches above, loose matches on Subject: below --
2007-04-08 23:22 Dana How
2007-04-09  0:04 ` Junio C Hamano
2007-04-09  0:18   ` Nicolas Pitre
2007-04-09 17:38     ` Shawn O. Pearce
2007-04-09 18:30       ` Nicolas Pitre
2007-04-09 18:40         ` Shawn O. Pearce
2007-04-09 19:11           ` Dana How
2007-04-09 19:33             ` Nicolas Pitre
2007-04-09 21:38               ` Dana How
2007-04-09 23:22                 ` Nicolas Pitre

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20070501050633.GZ5942@spearce.org \
    --to=spearce@spearce.org \
    --cc=danahow@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=junkio@cox.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.