Re: [PATCH 07/10] rev-list: call new "filter_skip" function

From: Christian Couder <chriscool@tuxfamily.org>
Date: Apr 2, 2009, 04:23 UTC
Message-ID: <200904020623.40908.chriscool@tuxfamily.org>
In-Reply-To: <alpine.DEB.1.00.0904011631300.13502@intel-tinevez-2-302>

Hi,

Le mercredi 1 avril 2009, Johannes Schindelin a écrit :

Show 26 quoted lines

> Hi,
>
> On Wed, 1 Apr 2009, Christian Couder wrote:
> > Le mardi 31 mars 2009, Johannes Schindelin a écrit :
> > > Hi,
> > >
> > > On Tue, 31 Mar 2009, Christian Couder wrote:
> > > > Le lundi 30 mars 2009, Johannes Schindelin a écrit :
> > > No, you want to _look up_ sha1s.  And struct decorate is not about
> > > storing objects, but to attach things to objects.
> >
> > The problem is that I don't have any object to attach things to when I
> > read the bisect skip refs. I just need to store the sha1 from the skip
> > refs in some sha1 container.
>
> I see, so you do not want to parse the commits just to register them as
> skipped.
>
> Fair enough.
>
> But I still think that a hashmap/set would be better suited.
>
> In any case, it should be refactored into something usable in all of
> libgit.a.  You are basically duplicating the grafts code in commit.c,
> sharing that shortcoming that your code would be static again, not
> encouraging reusage.

I agree that binary search functions and related code should be refactored. That's why I added the "Refactor binary search functions" task to the Janitor wiki page (http://git.or.cz/gitwiki/Janitor) a few weeks ago.

I will have a look at that, and perhaps, after that, I will try to make things less static, but I think these are some old problems in the code base, so new developments should not be hostage of them.

Best reagrds, Christian.