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/
