[Xapian-discuss] External Source Post List
Olly Betts
olly at survex.com
Wed Feb 21 20:18:35 GMT 2007
On Wed, Feb 21, 2007 at 12:06:00PM -0700, Rusty Conover wrote:
> Here are links to the patches:
>
> http://www.infogears.com/patches/externalsourcepostlist.patch
OK, I found my comments on the previous version you sent:
http://article.gmane.org/gmane.comp.search.xapian.devel/742
I think you've since addressed all of those except:
* ExternalPostingSource should derive from RefCntBase and then you can
use the same reference counting mechanism the other API classes do.
So instead of:
Xapian::ExternalPostingSource *external_source;
you'd have:
Xapian::RefCntPtr<Xapian::ExternalPostingSource> external_source;
Then all the reference counting happens as if by magic!
There are a few places that could do cleaning up a little - one hunk
just adds several blank lines to a function, and another adds a trailing
comma to an enum.
The ExternalPostingSource class and its methods ought to have
documentation comments so they are documented in the API documentation
which doxygen generates. These look like:
/** Brief description.
*
* Longer description.
*/
There's various markup you can use - see the existing documentation
comments for examples, or see the doxygen manual.
It would also be good to have some simple feature tests for the new
feature. A good test to copy is probably "matchfunctor1" in
"tests/api_db.cc".
This also won't apply cleanly to SVN HEAD, if only because I've reworked
the build system to be mostly non-recursive.
I can probably cope with doing some of the tweaking, though it might be
a while before I get to it. The documentation comments and test cases
are probably the most useful thing for you to provide, as they're the
hardest for me to fill in.
> http://www.infogears.com/patches/externalsourcepostlist-perl.patch
I've not looked at this in much detail, but adding the ability to pass
"check_at_least" is fine. I think it's only not wrapped because the
Search::Xapian wrappers pre-date the addition of check_at_least.
It's great that you seem to have wrapped MatchDecider such that it
can be subclassed in Perl (unless I misunderstood the patch). Several
people have asked about that in the past.
I'll see if I can merge the non-externalsource parts of the patch soon
and make a new Search::Xapian point release (the externalsource parts
need to wait for xapian-core).
Cheers,
Olly
More information about the Xapian-discuss
mailing list