One more thing I think we forgot about a while back is what to do about macros. I am still of the opinion that we need to be very careful about anything that gets exposed to users (Models & Macros) because those are the things we have to support long term and may not be able to change.

I haven't played with the 2 new tag macros, but should probably review them and make sure they are what we want.

-- Allen


Elias Torres wrote:

Allen Gilliland wrote:
I have a couple more very small things ...

1. I would prefer it if the new tables are prefixed with "roller_".
Obviously this has no functional benefit, but all things being equal I
think it's better if all of the roller tables are identified as such.
This has no downsides and definitely makes it easier on users who happen
to be mixing tables from many apps in the same database, so I see no
reason not to do it.

done.

2. We don't need the TagManager interface anymore right?  So it can be
deleted?

done.

-- Allen


Elias Torres wrote:
Allen Gilliland wrote:
Elias,

I am still playing around with the latest tagging code in the trunk, but
so far everything looks great.  I have a few more small items that I
think are worth doing, and one or two optional ones we can think
about ...

1. I think we should allow for configurable values for a couple things.

  - # of allowed tag intersections.  you hard coded 3, but i think this
should be configurable.  this is enforced in WeblogXXXRequest.
  - maximum length value returned from getPopularTags().  right now you
just accept the value passed in, but i think we should do a simple check
to make sure the value passed in doesn't exceed some configured max.
this is enforced in WebsiteData and SiteModel getPopularTags() methods.

2. You don't need a getTags() method in SiteModel, but it should be in
FeedModel.  Since the SiteModel is stateless there is no need for that
method, it's already available in the PageModel and you should add it to
the FeedModel as well.

Those are the only ones I have so far which I think should be done.
There are a couple more optional ones which may be worth doing as well,
but aren't necessarily tied to tagging ...
Done.

Reply via email to