Re: New addressbook backend.



[...]

> I think you're on the right path; I didn't mean to come across as
> wholly negative.  Here are my comments:

Yeah, no worries I was curious as to what you think is required to get the
code to where it needs to be.
>        * The rename from BackendResult to Result is good.  I'd go
>        further and call a Result a Match; as far as I can tell from
>        your code, this was the intent, and it seems to map everywhere I
>        see the Results being used.  So Result -> Match, ResultSet ->
>        MatchSet, etc.

I had trouble with Match, mainly because it was already used in Dashboard
at the time, and was slightly different from result.  With the new patch I
think that the old match and match filters have disappeared completely
(been sucked in to the resultsets) so this shouldn't be a problem.
>        * Matches should have types, so that you can map a type to a
>        renderer:
>
>        	Match m = new Match ("BugzillaBug");

[...]

I put this information in the result sets as a category, as each result
from a backend had the same category.  The new patch groups results by
category as well, so that should be easy enough to manage in terms of
rendering.  The rest of the stuff you mention on rendering fits pretty
much with how I envisaged it, so I'd hope that the internals can handle
it.
>        * Match fading.  I'm glad you are thinking about this.  We need
>        to get the match grouping and relevance filtering working better
>        before we can tune the fading, though.
>
>        Having match types will make grouping easier since we can
>        cluster all of the matches of a given type in one area.
>
>        Getting relevance-based filtering working is a matter of
>        tuning.  Right now almost all backends return the same level of
>        relevance for a given match, and there is no relevance
>        attenuation happening when we cluechain.  Both of these need to
>        get fixed, and that will be a gradual process.

Not sure we would need to attenuate with cluechaining, but that's easy
enough to discuss/implement as desired.  Again, the new patch is designed
to handle (a form of) grouping and match fading, although I've heard some
comments that it doesn't fully work yet.  I'll be playing with that later.
[Grouping]

>        In order for this to happen, though, we need to maintain
>        information about the structure of the frontend object all the
>        way through to the backend.  This is not possible with our
>        current setup; I think it is okay to treat this as a mid-term
>        problem though.

Hmm... I'd actually say that the problem is that you need to understand
the frontend object, rather than just pass the data around intact.  A
cluepacket has no idea what it's passing in today, apart from 'clues'. 
Suspect that this is a bigger disussion.
>        * Coding style.  Do you think you could make your code look like
>        the rest of the dashboard code?  Your stuff is absolutely great,
>        but the formatting doesn't match.

Yeah, this is my bad.  I didn't find the coding standards until later on
in the day, and although I changed a fair bit of it I think that there are
some outstanding issues (spacing around parnethesis is a biggie).  I'll
change it in all my new stuff, and back-port when I hit the older code.

> Nat

Cheers,
Jim.
-- 
Jim McDonald - Jim mcdee net





[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]