On 6/18/07, Allen Gilliland <[EMAIL PROTECTED]> wrote:
Some comments after looking at the latest code on this ...
1. I'm not really liking that the RollerContext object has become a
DatabaseScriptProvider. I think that functionality should be separated
out into its own class or something. I think we were guilty of putting
way too much logic in the RollerContext before and we should try and
avoid that.
2. I still think the BootstrapFilter should be simplified even further.
We want to do as little work there as possible.
3. Your struts2 actions are ApplicationAware and are pulling things out
of the servlet context, to me that's a very confusing way of managing
access to that database script provider object. Why does that class
need to be a singleton anyways? Why wouldn't the struts2 action just
instantiate that class when it's actually going to use it?
4. Unless you have a good reason, I'd really like to keep all the
struts2 actions extending the UIAction class rather than directly using
ActionSupport.
Overall I'm still feeling like this code is lacking an overall
cohesiveness and the code doesn't really match the workflow of doing an
installation and bootstrapping. So I think this still needs some work.
Thanks for the comments and I've got no objection to any of your
suggestions. I'm working removing pretty much all logic from the
BootstrapFilter and moving it to a controlling Struts action so we can
have some sort of cohesive page flow instead of the dead-enders we
have now.
- Dave
