> What happened in r6980?

that was my bad. I converted the file from MSDOS line endings to unix line 
endings as the rest of the code base. Unfortunately that was no the only thing 
I did - so the changes got hidden by that. Sorry for that :(

1. I am not sure I completely understand what you want. Right now there are two 
ways to run codecheck:
a) it will get run on files that are compiled when debug is enabled, but only 
if those files have changed.
b) make codecheck. It only runs over files that has issues or that have 
changed. 

That seems like reasonable behavior to me. Do you disagree?

2. Look into that. I think there might just have been a typo - I changed the 
testsuite around to automatically collect all tests and run them after each 
compile (release and debug). I really want to get away from all those 
conditionals in the cmake files as much as possible to unify everything. 
Building and running the tests with your new function works now. I renamed it 
to wl_test, as a testsuite is the collection of all tests.

3. 
a) the wl_strict option can be killed imho. Also the other changes you suggest 
there are reasonable. I left this TODO in the code for you to remove.
b) all tests are run in test_economy. I added manual failures in some of the 
test cases and they were all caught. I removed the TODO.
c) #TODO: (code review): Oh, and another thing; do we want to specificy 
dependencies for SDL/Boost/et al for each library? Does this make dependencies 
clearer and explicit or is it overkill?

I think it makes sense to specify the dependencies as explicit as possible. for 
example, if sound_handler has a dependency on SDL_GRAPHICS something is fishy. 
So yes, I think this makes sense.

The way I planned to do this is to add USE_SDL USE_SDL_GRAPHICS and so on to 
the wl_library() function (and it's derivatives, like wl_test()). I already did 
this for minizip. 




-- 
https://code.launchpad.net/~widelands-dev/widelands/cmake-reworked/+merge/222455
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/cmake-reworked 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