← Back to team overview

widelands-dev team mailing list archive

Re: [Merge] lp:~widelands-dev/widelands/autosave_on_objectives into lp:widelands

 

Review: Needs Fixing

I pushed my review of this branch in r6613. You can find all my comments by grep-ing for #cghislai. I changed some nits around (see the diff) and added a short comment when I found it appropriate - feel free to just delete them again, they are for your eyes only :). I did not touch anything that would affect the logic because you are more suited to do this as you know your code better.

I think the code looks very reasonable already. The biggest issues is that we now have wl.Game():save and wl.game.save_game - the first one is only used in the persistence.wmf test (see tests/lua/persistence.wmf/scripting/init.lua). I think you should move your logic to the method that is already there and make sure that its new features are exercised in the persistence tests - comments are in the source code as well.
-- 
https://code.launchpad.net/~widelands-dev/widelands/autosave_on_objectives/+merge/174546
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/autosave_on_objectives.


References