Adam, Thanks for the updated patch. It looks good and has been committed as revision 12221.
Comments are inline. >> * In general we don't want to commit commented out code. ... >> So if the code you commented >> out isn't useful for your ongoing work, please go ahead and remove >> it. > > Done. And in general I totally agree, so I'm glad you mentioned this! > Unless we need to support deprecation of APIs and such, the source > control change log might even be sufficient for a historical record > of what happened and why. Agreed. Thanks for the descriptive source controle comment you provided below. > I also removed a couple of member variables that weren't referenced > anywhere (scheduleMeetingOnHoliday, daysForCalDefinition) as well as > their corresponding hibernate mapping and DML/DDL in the sql scripts. Great- thanks for the cleanup. > I thought it might even be cleaner just to exclude the comment, too. > I'm happy to add it back if you want. Excluding it sounds good. > I didn't have time to evaluate the possibility of completely removing > the SYSTEM_CONFIGURATION table.. We can revisit this after wrapping up the configuration related stories. > Ah, good call. I got rid of my new join() in favor of Apache Commons' > StringUtils join() method. Great! > I started refactoring StringUtils, but it > was a slippery slope. :) Yes, these things can involve more work than you might think at first. > Here's a summary of the changes I made. This might be appropriate for > the commit log. I'm a fan of verbose version control commit logs, > since that's where I often look for answering those "why did they do > that?" questions. Agreed. Your summary has been used for the commit. Cheers, --Van ------------------------------------------------------------------------- SF.Net email is sponsored by: Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
