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 ...
I will do those.
3. Enforce strict urls. This is more of an overall approach type
question to url handling, but right now we allow for opportunities where
unsupported data is put into a url and instead of rejecting the url we
simply ignore the parts we don't support. This may not be the best idea
depending on how you look at it. Since technically we don't want people
to try and do /weblog/tags/foo+bar?cat=MyCat then it makes some sense to
reject those urls as 404 or possibly a 400 Bad Request. Obviously this
isn't really a problem, it's more of a question of best practice.
Correct. I believe that if it's a bad parameter is a bad request but if
it's in the path, then 404. I totally agree we shouldn't be ignoring
stuff and should be reporting it accordingly.
4. Optionally enforce a max entries per page setting. We had briefly
talked about this during the 3.0 release, but it didn't end up getting
done. In any case, I think this is a good thing to be a site-wide
configurable setting so that the admin can define the maximum number of
entries to show on a page, and the default Roller value can be
"unlimited". On large scale sites like blogs.sun.com people make some
rather silly decisions, like setting the entry display count to "9999",
and so enforcing a max value seems quite reasonable to me. I had
planned to do this for 3.0, but it slipped by me, so I can take this as
an item for 3.1.
No problem. Is it a small change?
yeah. it's basically adding the property to the config and then
enforcing it in a couple of the pagers. i will do it today.
-- Allen
-- Allen
Elias Torres wrote:
Hi Guys,
I consider tagging to be code-complete, but definitely not bug-free. Now
I'll start working with DB2 to make sure we are compatible in the SQL
queries/structure and keep on testing. Please review/test it to make
sure I'm not missing anything really important.
Thanks.
-Elias
