On Wed, Feb 22, 2006 at 10:26:13PM +0000, Dave Page wrote:
My personal interest is in disabling the prefixes
entirely. I'm running
into situations where Category Foo is just a @REDIRECT to Foo, or
vice-versa, and the same applies to Locales. I don't see any reason to
prefix the node names with "Category" or "Locale" when that
information
is presented by whether the node is in category "Category" or category
"Locale".
Okay, thanks for that explanation. I've got a few comments on the patch
itself (
http://dev.openguides.org/attachment/ticket/81/ticket_81.patch)
trunk/lib/OpenGuides/Config.pm
new lines 87-88 - if Config::Tiny doesn't accept spaces, presumably this
default shouldn't have spaces either for consistency. Will the code
always append trailing spaces, or only if there isn't already one?
trunk/lib/OpenGuides.pm
old lines 150-159 - I'm not sure why you're removing this. Can you
explain?
new lines 506-516 - surely you need to set $type_prefix depending on the
value of $args{type} rather than just setting it to
$config->category_prefix?
I assume that the test suite passes all tests with this patch, and
you've tested the functionality you've added? In fact, could you add a
test for the functionality itself?
Cheers,
Dominic.
--
Dominic Hargreaves |
http://www.larted.org.uk/~dom/
PGP key 5178E2A5 from the.earth.li (keyserver,web,email)