2008/6/3 Lea Wiemann <lewiemann@xxxxxxxxx>: > Rafael Garcia-Suarez wrote: >> >> Finally, to avoid forking git-rev-parse too many times, cache its >> results in a new hash %parent_commits. > > I'm not too happy with this: > > 1) Minor point: I'm working on caching for the backend right now (IOW, > basically what you're doing, just centralized in a separate module), so > you're essentially duplicating work, and you're making it (a little) harder > for me to refactor gitweb since I have to rip out your cache code. Those > few lines won't hurt, but in general I suggest that nobody make any larger > efforts to cache stuff in gitweb for the next few weeks. Sorry for that. I failed to consider that your work was applying there as well. > 2) Major point: You're still forking a lot. The Right Thing is to condense > everything into a single call -- I believe "git-rev-list --parents --no-walk > hash hash hash..." is correct and easily parsable. Its output seems to be > lines of > hash parent_1 parent_2 ... parent_n > with n >= 0. Can you implement that? It would be very useful and also > reusable for me! Ok, I see what you mean -- but I suppose that's the kind of information that would be nicely abstracted out somewhere. At least I would put it in a sub. And that sub would fit perfectly in your caching module, too! Looking at the source, a generic git-rev-list wrapper could be used. If you want I can produce a patch against your version, but I'm afraid I'll end up being a bit short on time, so I might be slow to do it. > P.S.: I believe that the usual way to post follow-up patches is to label > them [PATCH vN] for N >= 2 in the subject (since "take 2" shouldn't be part > of the commit message), and to send them as In-reply-to a message in the > original thread -- just provide git-send-email with the Message-ID of the > message you want to reply to. </nitpick> > Ok, noted. Also, next time, I'll send smaller patches. -- 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
