code for FLUID-2925 and FLUID-2922

Laurel A. Williams laurel.williams at utoronto.ca
Fri Jun 19 15:30:07 UTC 2009


Thanks for taking the time Colin and for your comments.

I just created http://issues.fluidproject.org/browse/FLUID-2963 to 
address point 1
http://issues.fluidproject.org/browse/FLUID-2952 - was already waiting 
for me to get to - I'm quite excited to spend a little time 
investigating php unit test frameworks.

Do you prefer that I do these before checkin or can I patch them after 
the code gets into the svn?

Laurel

Colin Clark wrote:
> Hi Laurel,
>
> Nice work, and the code looks good. Couple of comments:
>
> 1. At the end of the file, you've got an else block with a comment: 
> "//TODO what to do if the var wasn't set?" In this case, you should 
> return an appropriate HTTP error. 400 is probably right.
>
> 2. There's enough fiddly string manipulation code here that you may 
> want to cook up some unit tests for your core logic. It should be 
> fairly straightforward to do.
>
> Colin
>

-- 
Laurel A. Williams
Adaptive Technology Resource Centre
University of Toronto