Nooo, not the beer! I liked the beer! :(

But thanks for the review. The small stuff (renaming, etc.) is done, what is 
left are the bigger changes. Open problems in no particular order:

1) Making WorkersQueue more similar to WaresQueue and replacing the user 
interface should be no problem. I will try to share as much code as possible 
between the queues respectively the interfaces.

2) The problem with higher-ranking workers seems to be a bug (or inconsistency) 
in the code. While warehouses check for an exact match to fulfill the request, 
the IdleWorkerSupply uses can_act_as(). This explains the strange behavior I 
encountered. For normal (worker-)workers I would prefer the can_act_as() 
approach while barracks should match exactly. What do you think about a flag in 
the request which describes whether the worker has to match exactly? Or maybe 
expand the "Requirements" for requests (new RequireExactWorker class or so)?

3) In production_program.cc:220 you requested a for-each loop. I first thought 
this would be a problem with the if() inside the loop which checks the 
iterator. But now I am wondering: Can this if() ever be fulfilled? When I am 
not missing anything the loop should always end earlier. So remove the 
if()-part and use a for-each loop?

4) What does "NOCOM" mean? I just can't figure it out. And what is the 
difference to "TODO"?

5) You increased the packet version for the serialization functions and they 
are now checking for a range. What is the idea behind it? Increase the number 
on every modification of the file but accept older versions until the method 
itself changes?

So much for now.
Thanks for the link to the documentation. I tried the parameter singlehtml 
before, seems that this does not include the code documentation. A full html 
worked fine.
-- 
https://code.launchpad.net/~widelands-dev/widelands/casern_workersqueue/+merge/309763
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands.

_______________________________________________
Mailing list: https://launchpad.net/~widelands-dev
Post to     : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help   : https://help.launchpad.net/ListHelp

Reply via email to