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