widelands-dev team mailing list archive

Thread
Date

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

To: mp+274832@xxxxxxxxxxxxxxxxxx
From: SirVer <SirVer@xxxxxx>
Date: Mon, 02 Nov 2015 08:05:07 -0000
In-reply-to: <20151018205822.16089.50641.launchpad@ackee.canonical.com>
Reply-to: mp+274832@xxxxxxxxxxxxxxxxxx
Sender: bounces@xxxxxxxxxxxxx

Review: Approve

> Renaming "outputs" to "occupants" won't work, because this is a feature of production sites in general and not just militarysites.

Add a TODO that militarysite should not be a productionsite? Outputs is really weird for militarysites. 

Okay, done with the review. I added a bunch of comments, but found the code very clean and overall a huge improvements. Please have a look over my comments, but I think most should just be converted to bugs and/or todos and this should be merged rather quickly.


Congratulations on this branch - that was a ton on work and a daunting task. Thanks for doing this!




-- 
https://code.launchpad.net/~widelands-dev/widelands/one_tribe/+merge/274832
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/one_tribe.

References

[Merge] lp:~widelands-dev/widelands/one_tribe into lp:widelands
From: GunChleoc, 2015-10-18