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? > > -- 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 >
