On 5/1/07, Nicolas Pitre <nico@xxxxxxx> wrote:
On Tue, 1 May 2007, Shawn O. Pearce wrote: > Dana How <danahow@xxxxxxxxx> wrote: > > On 4/30/07, Shawn O. Pearce <spearce@xxxxxxxxxxx> 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@xxxxxxxxx +1 650 804 5991 cell - To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html