Code review comment for lp:~widelands-dev/widelands/autosave_on_objectives

Revision history for this message
SirVer (sirver) wrote :

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.

review: Needs Fixing

« Back to merge proposal