From: Dana How Date: Tue, 01 May 2007 17:58:52 GMT Subject: Re: [PATCH 4/8] git-repack --max-pack-size: add fixup_header_footer() Message-ID: <56b7f5510705011058y89e4aa4p8f8b7eccde30af78@mail.gmail.com> In-Reply-To: On 5/1/07, Nicolas Pitre wrote: > On Tue, 1 May 2007, Shawn O. Pearce wrote: > > Dana How wrote: > > > On 4/30/07, Shawn O. Pearce wrote: > > > > 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)? > > > Actually I didn't just copy it, I tried to rewrite it for my use > > > as well as the fast-import.c use (note there is a 3rd copy > > > in some *index*.c file which I didn't try to merge in yet). > > > However I didn't yet put it in a new file or change fast-import.c > > > to call it since I wanted to change as little as possible. > > ... > > > I agree with all your arguments. I had several reasons > > > to avoid extra rearrangements/refactorings: > > > (a) First patch to git, not previously known to me; > > > (b) I prefer to separate new functionality from "clean-up" work; > > > > A really good reason. ;-) > > > > But I'd still rather see it done right the first time, then done > > partially (copied) and wait for someone to clean it up later. > > Sometimes that cleanup doesn't happen. > > Well I intended to do more cleanups in the pack code eventually. That > included the index writing and pack header fixing. But I was expecting > for the pack splitting changes to go in first as it is likely to impose > some requirements of its own. It is then easier to have a proper > interface common to all users after everything is in place. I was in the middle of creating pack-write.c at Shawn's suggestion. It will only contain fixup_header_footer(), to be called by fast-import.c and builtin-pack-object.c. index-pack.c also has readjust_pack_header_and_sha1(), which is compatible except it doesn't close the file. I was going to leave it alone for now. This new file should be the logical place to put other common pack-writing-related things. Please barf now if you don't think I should do this tiny refactoring at this point. > > > I'd have no problem submitting a follow-on patch later containing > > > some clean-up work if you & NP clear it, so I know I won't have > > > problems from (d). Note I had to completely rewrite this patch > > > when NP submitted some of his pending stuff. > > > > Yea, hazard of working in this part of the code when Nico is > > also active. My own sliding mmap stuff was written twice too, > > for the same reason - Nico doing much needed improvements right in > > the same spot as I was working, at the same time. > > Well well. OK I'm used to be considered as the bad guy anyway. ;-) You *did* tell me about your upcoming patches as I recall ;-) Thanks, -- Dana L. How danahow@gmail.com +1 650 804 5991 cell