Allen Gilliland wrote: > ok, i finally got around to looking at this a little bit last week and > the following is my review. My general feeling is that the > implementation is not complete and needs some more work before I would > be ready to commit it to the trunk. My reasons are ... > > 1. The data model is very simple and doesn't include anything from the > discussions we had with David Levy on the mailing list. It's still > possible that we want to keep the data model this way, but I have not > made my own decision about that yet and I think it needs more discussion.
Let's have more discussion. I re-read the old thread and found this [1] to be David Levy's latest table definition: create definedtags as ( author_name char(32) entry_name (or id?) , char(128) user_tag_name, char(64) normal_tag_name, char(64) entry_date datetime date_created datetime ) I can start adding this to the code base for now or should I wait for more discussion? > > 2. The backend code added to the WeblogManager doesn't look great. I > don't like that some of the methods return things like Object[] arrays > rather than processing that data and putting it into a more intuitive > Object. Some methods only partially implement what the method suggests > it does. I was looking in [2] and [3] and I could not find anything that returned Object[] arrays, can you please point me to the cases you are referring to so I can fix. Also, can you let me know which methods are partially implemented? Thanks. > > 3. There is no consideration put towards using tags in xml feeds. This > may be a good thing at first, but in the original proposal we talked > about having xml feeds based on tags, so I expect something. I'll get on it, sounds like must-have to me as well. > > 4. There is no consideration for performance or caching. Personally, I > would like to at least see some kind of documentation about how the > implementation plans to maintain good performance. If we haven't even > thought about that then we have a problem. I would like help on this area. What are your thoughts? > > 5. The UI isn't exactly well done. The tags page doesn't have its own > tab, so it looks weird when you are on the tags page and have the Main > and Planet tabs showing. And I don't believe the UI had any way to > constrain the tags view to a specific weblog. I'll work on making this a tab and think about how to view tags within a specific blog. Although I think that's a blog UI/template issue and not the dashboard area. > > 6. No configuration options. There is no easy way to enable/disable > tagging support like we discussed and there are no controls for how the > tagging support will work if it is enabled. I think considerations like > items per page, max tag combinations allowed, etc are worth making > controllable. I'll take a stab at this. > > So, i think this is a step in the right direction, but i would like to > see things flushed out a bit more before we commit them to the trunk. i > would also like to continue a bit more of our discussion on the tagging > data model because i think that is the most crucial piece of the puzzle > for tagging. Makes total sense. Now a question from me.. Should I keep working on the same branch to apply these changes? or start with a fresh branch from the trunk? or wait until Allen checks in the new backend code? options, options.. -Elias [1] http://www.nabble.com/forum/ViewPost.jtp?post=2361138 [2] /roller_2.1_tagging/src/org/roller/business/WeblogManagerImpl.java or http://tinyurl.com/kte6t [3] /src/org/roller/business/hibernate/HibernateWeblogManagerImpl.java or http://tinyurl.com/fqllf
