widelands-dev team mailing list archive

[SirVer <SirVer@xxxxxx>]

> 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.