Hey John.
Looks like you've gotten the nice urls working, great job man. It will be great to have this before 1.0 is shipped since it will make our urls a lot nicer looking.
I had a few comments about the code changes. You don't follow the variable naming convention of the appdb. We have some prefixes for variables that we use to keep the types of variables straight, you can find these in the coding standards file in the root of the appdb tree.
The code in 0005-matchByVerbAndTerm.php-view-that-calls-into-the.patch.txt looks like it is using query_appdb() to perform a direct query. The indexing code also appears to do this. We prefer using the query wrapper function query_parameters() as this performs escaping of input parameters and helps to reduce the possibility of sql injection vulnerabilities.
I noticed the code to perform the grouped inserts. What is the performance difference between perform the inserts individually vs. grouped together? If we run the indexer daily and its relatively quick we may be able to drop the complexity of worrying about how many inserts we can group together.
0005-matchByVerbAndTerm.php-view-that-calls-into-the.patch.txt appears to be generating output for display. It would be useful to have some comments at the head of the file to describe what the file is used for and why it would need to conditionally output lists when don't find a object page match.
Thanks, Chris
On Mon, Apr 7, 2008 at 5:05 PM, John Klehm xixsimplicityxix@gmail.com wrote:
Give nice urls a whirl at: http://appdb.klehm.net/app/%22yoursearchtermhere"
Search Examples: http://appdb.klehm.net/app/adobe http://appdb.klehm.net/app/visual
Direct Link Examples: http://appdb.klehm.net/app/photoshop http://appdb.klehm.net/app/command-conquer-red-alert-2
See the nice urls being used internally: http://appdb.klehm.net/appbrowse.php?iCatId=5
Right now as it is I have it indexing only the app family names; no versions or vendors get any love.
There is one piece of functionality missing before this can go in and that is to do an index of a new record upon creation.
If anyone wants to test just apply the patch set and then flip APPDB_ENABLE_NICE_URLS_INDEXER to true and then run: runIndexer.php. Be sure to flip enable indexer back to false before doing anything else =P.
You also need to set allow_url_fopen = On in your php.ini. If this is deemed to be unacceptable then we have to functionalize the objectManager.php file so matchByVerbAndTerm can generate similar output. Using url fopen allows easiest integration.
After that you should be good to go to test.
Essentially less terms will work better and the matching "nice" name depends entirely upon whatever the maintainer entered for the app family name.
Let me know if any changes need be made otherwise ill send it in later this week.
Regards, John Klehm