Yep, sounds good for me too! Thx vlad
david blanchard <[email protected]> wrote: >Hi Vlad, > >Works for me. >Then how should we proceed ? We had estimated 4 days for the 2.1 (front end >missions), and if you start missions on tuesday, we'll have 3 days before >the end of the iteration. What can be done in 3 days, any way you can split >the mission work so I can start a first review of the missions at the end of >the iteration ? > >For the 2.2, I still can't un hackit locally, can I grab you on skype to >look at this together ? > > >> -----Message d'origine----- >> De : [email protected] [mailto:[email protected]] De la >> part de Vlad Dragu >> Envoyé : lundi 1 novembre 2010 09:30 >> À : [email protected] >> Objet : Re: [Farsides] [Code] [HackIt] Review alpha2.2 >> >> hi Xavier, >> I looked at the code review changes. >> I estimate about half a day - a day of work. i can do it this Monday >> but >> this means that i will push back the missions on Tuesday. >> will that be ok? >> >> vlad >> >> On 10/30/2010 5:45 PM, Xavier Antoviaque wrote: >> > Hi Vlad, >> > >> > I've spent some time on the code to familiarize myself with the >> changes. >> > There is a lot to go through, so I haven't looked at everything in >> > details, but I think I've got a good overview of how things are >> > structured now. >> > >> > I really like it, I think it's taking a good shape. There is still >> some >> > tuning to do (there will always be some), but a good bulk of work is >> > behind now. Congratz! >> > >> > Here are some comments/changes/questions - I'll let you read them, >> let >> > me know what you think. >> > >> > === >> > >> > - Dataobjects auto-generated for Ajaxim: Zajaxim_friends.php& ajaxim >> > directory => is it remnants from trying to add ajaxim in the past? >> If so >> > they need to be removed >> > >> > - Dataobject files need to be chmod 644 >> > >> > - Missing a merge from lp:hackit >> > >> > - Same issue as David - https://bugs.launchpad.net/hackit/+bug/668243 >> . >> > I've updated a few files on lp:hackit, but it seems that it's missing >> > the SQLs for the updated database structure (see the ticket) >> > >> > - Missing an update of the CHANGELOG >> > >> > - Need to add instructions to regenerate data objects >> > >> > - Format of the class names: "OneTwoThree", not "one_two_three" >> > >> > - Why did you separate User and the users data object? You add >> yourself >> > a lot of trouble - you could simply add the methods directly on the >> > dataobject, and expose it directly, no? But maybe there is something >> I >> > didn't see. (And same question about the Site and GameSession >> classes, >> > actually) >> > >> > - Instead of individual arguments about the current site directly >> > attached on GameSession, should attach a Site as >> > GameSession->current_site, containing: >> > * public $web_address; >> > * public $encoded_address; >> > * public $proxy_page; >> > * public $hack_probability; >> > * public $scan_probability; >> > >> > - Need to remove the part "TO DO remove at end of 2.0 iteration" ;p >> > >> > - You could move more code to the dataobjects themselves - basically >> > code that only require to manipulate dataobjects themselves should be >> > considered, especially if it could be useful to different objects. >> > >> > - I missed that one in the UML schema: code related to hacks and >> scans >> > should be isolated on other objects, here it makes the game session >> > class HUGE. Why didn't you put it in the action observers? You could >> > actually isolate a good portion of the GameSession logic into smaller >> > dedicated objects this way. >> > >> > - Btw, you should put all the files containing the classes for the >> > action observers in a dedicated subdirectory of classes/ . And >> congratz >> > btw for the mechanism here, it works very gracefully. >> > >> > - prepare_address() should be on the Site class >> > >> > Xavier. >> > >> > _______________________________________________ >> > Farsides mailing list - [email protected] >> > >> > Wiki: http://farsides.com/ >> > List: http://farsides.com/ml/ >> > Forum: http://farsides.com/forum/ >> > Ideas: http://farsides.com/ideas/ >> > Chat: http://farsides.com/chat/ >> > >> > >> >> _______________________________________________ >> Farsides mailing list - [email protected] >> >> Wiki: http://farsides.com/ >> List: http://farsides.com/ml/ >> Forum: http://farsides.com/forum/ >> Ideas: http://farsides.com/ideas/ >> Chat: http://farsides.com/chat/ > >_______________________________________________ >Farsides mailing list - [email protected] > >Wiki: http://farsides.com/ >List: http://farsides.com/ml/ >Forum: http://farsides.com/forum/ >Ideas: http://farsides.com/ideas/ >Chat: http://farsides.com/chat/ _______________________________________________ Farsides mailing list - [email protected] Wiki: http://farsides.com/ List: http://farsides.com/ml/ Forum: http://farsides.com/forum/ Ideas: http://farsides.com/ideas/ Chat: http://farsides.com/chat/
