[redundant widelands* in file and library names]
I agree about removing them. They were sometimes added in the past to work 
around having the same names in system header files. But since our include 
paths are now much cleaner and we always include the full local path renaming 
should not be problematic. Just make sure to use bzr mv so that bzr pick up on 
the renames.

> A single header file in its own library is a little bit of an overkill. Then 
> again it might be good for the initial refactoring and modularization and we 
> can merge libraries that are often used together into one.

I disagree - libraries should be small, logical units. And sometimes a single 
header with one feature (like container_iterate) doesn't really belong anywhere 
else. With the build_deps.py script it is also no too inconvenient to keep the 
build files up to date. The only think that is unfortunate is that cmake does 
not support header only libraries - so I had to add a bunch of dummy .cc files. 
For that reason I can see bunching up small libraries into bigger catch-alls 
would make sense.

> Thanks for all the work. I really like the way the Widelands code is 
> developing right now: easier to understand, better structured and more fun to 
> work with :) 

I am glad that you approve of the changes. I feel I spend too much time 
refactoring (one_world is also just a refactor basically) and not adding enough 
new value to the project. Also the latest changes (especially this one) does 
not really make the live of developers easier - we now have to maintain our 
build files too instead of just dumping a .cc file someplace and it will get 
compiled. I still think this makes more sense in the long run - but only when 
we follow through and reduce coupling in the code base. I hope everybody will 
help out with this.

> I hope to contribute more in the future myself but currently I am a little 
> afraid to do big changes because everything is changed so quickly.

1) There are now plenty of 10-60 minutes task that can be done - peeling away 
ball_of_mud into smaller libraries or splitting logic into smaller, less 
coupled pieces. I also filed a bunch of bugs with tag cleanups 
(https://bugs.launchpad.net/widelands/+bugs?field.tag=cleanups). Most of them 
should be fairly quick too. Feel free to work on that :)
2) I maintained 2 huge changes (one_world and Guns wrapping of MapObjectDescr) 
in the last few months, and I was very surprised how well bzr handled merging 
of the huge changes. Sure, they were merge conflicts, but if you always start 
out with merging trunk when you sit down to work on anything, you are pretty 
fine.
3) Striving for smaller changes is always better - for example feel free to do 
refactorings in smaller pieces. For example, introduce a better API first and 
start using it in only some places and then change the users one by one in 
trunk later and only remove the old API when done completely is fine. I think 
we should move away from big branches as much as possible (of course it is not 
always possible). If you have something that you want to work on specifically, 
feel free to reach out and we can come up with a strategy that does not take 
you too far away from trunk at any time.

[compilers]
Teppo, thanks for fixing the gcc builds. I think we can now state that 
widelands compiles on gcc >= 4.7 and clang >= 3.3. I feel good about that and 
consider the c++11 transition a success. 

[older cmake versions]
I am usually not too concerned about debian, but others usually are. I will go 
ahead and merge this branch now as is, but I am fine with somebody adding one 
more macro that does target_include_directories and SYSTEM for never cmakes and 
adding a simple -I for all targets when running on older cmakes. I think this 
can happen in trunk. 

[Closing remarks].
This branch is a huge step forward for better code structure in Widelands. I 
also think that it was a wonderful collaboration example with 6 people 
participating in the discussion and 4 people contributing code. I wish we could 
have more of those branches in the future - if somebody has suggestions for 
procedure changes how to make that happen more often, I am very open.
-- 
https://code.launchpad.net/~widelands-dev/widelands/cmake-reworked/+merge/222455
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/cmake-reworked.

_______________________________________________
Mailing list: https://launchpad.net/~widelands-dev
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~widelands-dev
More help   : https://help.launchpad.net/ListHelp

Reply via email to