Fwd: request for review

View: New views
3 Messages — Rating Filter:   Alert me  

Parent Message unknown Fwd: request for review

by Justin Obara-3 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Oops sent this from the wrong e-mail address. Please see below.

Begin forwarded message:

From: Justin <justin.obara@...>
Date: November 4, 2009 11:20:41 AM EST
To: "Laurel A. Williams" <laurel.williams@...>
Cc: fluid-work List <fluid-work@...>
Subject: Re: request for review

Hi Laurel,

Please see my comments below.

- Justin
On 2009-11-04, at 9:41 AM, Laurel A. Williams wrote:

Hi Justin,

Can I impose on you for a few very small reviews?

Changes made to the code to aid in deployment (since you've test driven that twice now, I think you are best to check those)
http://issues.fluidproject.org/browse/FLUID-3346 - rename mysql script

That seems fine.

http://issues.fluidproject.org/browse/FLUID-3329 - update mysql script

I actually don't know too much about SQL, but it seems fine at a glance. You may want to run this by someone else though.

http://issues.fluidproject.org/browse/FLUID-3327 - create config.php and builder.php
appropriate for server and local deployment

There are two files builder.php and builder_local.php. They are only different by a minor change to "DISTANT_PATH". There is a question here of whether it is easier to modify the path since you may have to do this anyways on a local install or to rename and delete files. That being said, speaking to Laurel, she mentioned that by having the two separate files, it will make creating an install script easier.

http://issues.fluidproject.org/browse/FLUID-3326 - create config.php file that works for the server deploy

If possible, in config.php, it might be better to have all of the fields that need to be set, grouped close together. Currently the sets of information that needs to be configured are separated by a chunk that says not to touch it.



Two small css issues:
http://issues.fluidproject.org/browse/FLUID-3253 - ie 7 issue for css (won't fix - unless it comes back after we start to put the website packaging back on).
http://issues.fluidproject.org/browse/FLUID-3342 - spinner image missing

So you've just added it into the repository, that seems right.


Laurel
<laurel_williams.vcf>_______________________________________________________
fluid-work mailing list - fluid-work@...
To unsubscribe, change settings or access archives,
see http://fluidproject.org/mailman/listinfo/fluid-work



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

follow up review

by Laurel A. Williams :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Hi Justin

If you would like to do a follow up review, I've addressed your comments
about config.php in
http://issues.fluidproject.org/browse/FLUID-3188. The actual task was to
remove the SRC_FILE_NAME and MIN_FILE_NAME values from config.php and
replace with a dynamically created version of these file names (which I
did in postProcessor.php). While removing those values I re-wrote the
instructions on how to use the config.php file, so your suggestions came
in handy.

Laurel


>>
>>> http://issues.fluidproject.org/browse/FLUID-3326 - create config.php
>>> file that works for the server deploy
>>
>> If possible, in config.php, it might be better to have all of the
>> fields that need to be set, grouped close together. Currently the
>> sets of information that needs to be configured are separated by a
>> chunk that says not to touch it.
>

[laurel_williams.vcf]

begin:vcard
fn:Laurel A. Williams
n:Williams;Laurel
org:Faculty of Information, University of Toronto;Adaptive Technology Resource Centre (ATRC)
email;internet:laurel.williams@...
title:Accessibility Software Designer
x-mozilla-html:TRUE
version:2.1
end:vcard



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

Re: follow up review

by Justin Obara-3 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Hello,

That does seem to address my comments.

Thanks
Justin

On 2009-11-04, at 3:31 PM, Laurel A. Williams wrote:

> Hi Justin
>
> If you would like to do a follow up review, I've addressed your  
> comments about config.php in
> http://issues.fluidproject.org/browse/FLUID-3188. The actual task  
> was to remove the SRC_FILE_NAME and MIN_FILE_NAME values from  
> config.php and replace with a dynamically created version of these  
> file names (which I did in postProcessor.php). While removing those  
> values I re-wrote the instructions on how to use the config.php  
> file, so your suggestions came in handy.
>
> Laurel
>
>
>>>
>>>> http://issues.fluidproject.org/browse/FLUID-3326 - create  
>>>> config.php file that works for the server deploy
>>>
>>> If possible, in config.php, it might be better to have all of the  
>>> fields that need to be set, grouped close together. Currently the  
>>> sets of information that needs to be configured are separated by a  
>>> chunk that says not to touch it.
>>
> <laurel_williams.vcf>

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