code for FLUID-2925 and FLUID-2922

[Jacob Farber]

Hi Laurel,
Justin and I took some time this morning to review the php files in FLUID-2922 and 2925, and it looks fine - Justin told me your going to take advantage of PHP's native JSON parsing where necessary, which should help simplify the code.

Sorry for the delay,
Jacob


-----Original Message-----
From: fluid-work-bounces at fluidproject.org [mailto:fluid-work-bounces at fluidproject.org] On Behalf Of Laurel A. Williams
Sent: Friday, June 19, 2009 11:30 AM
To: Colin Clark
Cc: fluid-work List
Subject: Re: code for FLUID-2925 and FLUID-2922

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

_______________________________________________________
fluid-work mailing list - fluid-work at fluidproject.org
To unsubscribe, change settings or access archives,
see http://fluidproject.org/mailman/listinfo/fluid-work