oh dear - a rather large request for review

[Colin Clark]

Hi Laurel,

I've managed to get through your list of requested reviews, and don't  
have any specific comments about your fixes. Most of them were small  
issues, and your changes look just fine.

The one major change was your Ant scripts for managing the deploy  
process. I haven't had a chance to actually test drive the deploy  
process for Builder yet, but I can say that from a code perspective  
your Ant scripts are small, simple, and well-factored. Nicely done!

Reviewing a huge block of JIRAs like this can be pretty overwhelming.  
As a favour to code reviewers, could you perhaps sort your requests  
into two categories: 1) substantive code changes, and 2) minor tweaks?  
Some of these little issues can probably go without distinct code  
review beyond, say, running the unit tests. It also helps people to  
prioritize which issues are most important to look at first. That way,  
you might increase your chances of speedy review for the big ones.

I hope this helps,

Colin

On 25-Nov-09, at 11:23 AM, Laurel A. Williams wrote:
> It seems I should have been JIRA gardening more often because I've  
> got a whole pile of weeds here! Any chance I can get some reviews?
>
> Recent blockers:
> http://issues.fluidproject.org/browse/FLUID-3324 - re-arranged  
> directory structure of Builder to accommodate deploy structure.
> http://issues.fluidproject.org/browse/FLUID-3353 - review of deploy  
> script
> http://issues.fluidproject.org/browse/FLUID-3389 - use UUID for  
> directory name for temporary file storage
>
> Mysql scripting:
> http://issues.fluidproject.org/browse/FLUID-3329 - update mysql  
> script - justin reviewed but suggested I run this by someone else.
> http://issues.fluidproject.org/browse/FLUID-3354 - add another mysql  
> script to create the cache database for local deploy
>
> Retrieve data from build.properties:
> http://issues.fluidproject.org/browse/FLUID-3188 - get filename from  
> build.properties
> http://issues.fluidproject.org/browse/FLUID-3356 - remove hard coded  
> fluid version numbers
>
> Broken php tests:
> http://issues.fluidproject.org/browse/FLUID-3351 - changes to get  
> the php tests working after new php.ini installed. - See Laurel to  
> understand how to run the tests.
> http://issues.fluidproject.org/browse/FLUID-3350 - changes to php  
> tests due to directory restructuring.
>
> I asked Jacob to review these, but I'm not sure if he did it. I  
> don't have an email response, and I don't recall him saying it was  
> done.
> http://issues.fluidproject.org/browse/FLUID-3323 - fix link to fss  
> reset
> http://issues.fluidproject.org/browse/FLUID-3299 - remove hard coded  
> constant DIRECTORY_DIVIDER
> http://issues.fluidproject.org/browse/FLUID-3306 - investigate  
> relative path problem that Colin had when releasing Builder
> http://issues.fluidproject.org/browse/FLUID-3139 - replace windows/ 
> unix slashes in postProcessor.php
>
> Colin - I wasn't sure if further checking of this was required
> http://issues.fluidproject.org/browse/FLUID-3349 - We worked on most  
> of it together but I did a little work at the end on my own to get  
> the tests working.

---
Colin Clark
Technical Lead, Fluid Project
http://fluidproject.org