Hello, I applied to letor stabilisation project for gsoc. I'd like to use coming weeks to improve the workability of xapian-letor. For that, I'm planning to refactor code in current master and begin writing some tests for it. Before adding tests, I think it would be better if xapian-letor could be made consistent with how xapian-core is written. For that, I'd first like to restructure some directories. Here are a few things that I intend to do: 1. Placing all api headers together (e.g. ranklist.h, letor_internal.h etc) in xapian-letor/include/letor 2. Rewriting letor.h and placing it in xapian-letor/include so that it does a job similar to xapian.h. The current letor.h contains some methods for which it is not the appropriate file. 3. Creating subdirectories like "ranker", "features" etc. and placing appropriate files in them. 4. Fixing minor bugs that I encounter in completing this process. (For instance, including svmranker in Makefile.am, since it does not do so at present). 5. Ask for a review and merge changes to master. Is this the right way to go about it? Regards, Ayush -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.xapian.org/pipermail/xapian-devel/attachments/20160402/134e0e5e/attachment.html>
On Sat, Apr 02, 2016 at 06:27:02PM +0530, Ayush Tomar wrote:> Here are a few things that I intend to do: > > 1. Placing all api headers together (e.g. ranklist.h, letor_internal.h > etc) in xapian-letor/include/letor > > 2. Rewriting letor.h and placing it in xapian-letor/include so that it does > a job similar to xapian.h. The current letor.h contains some methods for > which it is not the appropriate file.I like the idea, but since these (1 and 2) get installed under /usr/include when packaged, I think "xapian-letor/include/xapian-letor/" and "xapian-letor/include/xapian-letor.h" are better from a namespacing point of view.> 3. Creating subdirectories like "ranker", "features" etc. and placing > appropriate files in them.This seems reasonable, so long as it's not taken to excess - if you end up with a lot of subdirectories, each with 1 or 2 files in, that would probably be harder to work with than a flat structure.> 4. Fixing minor bugs that I encounter in completing this process. (For > instance, including svmranker in Makefile.am, since it does not do so at > present). > > 5. Ask for a review and merge changes to master. > > Is this the right way to go about it?Sounds good to me. Cheers, Olly
On Sun, Apr 03, 2016 at 01:23:25AM +0100, Olly Betts wrote:> > 4. Fixing minor bugs that I encounter in completing this process. (For > > instance, including svmranker in Makefile.am, since it does not do so at > > present). > > > > 5. Ask for a review and merge changes to master. > > Sounds good to me.It's worth pointing out that the master branch does not contain a working version of xapian-letor, for the reasons discussed in the project description. So you are likely to need to incorporate code from v-hasu's branch in order to get anything useful, which is why we suggest structuring the project as described. A subset of the work that might be helpful is to bring in the external header files (what Olly is suggesting end up as xapian-letor.h and in xapian-letor/) and do the moving around and refactoring/splitting that you suggest. That should make for a small change that can be reviewed, and has the advantage of making clear the Letor API. (This then enables further pieces of work such as starting to write automated tests for the API, and may also highlight areas where the API is perhaps too closely bound to the tools that have been written so far.) J -- James Aylett, occasional trouble-maker xapian.org