Review: Approve
Your changes look fine. Just one question and an answer to your question in the
code below.
Diff comments:
> === modified file 'mainpage/templatetags/wl_markdown.py'
> --- mainpage/templatetags/wl_markdown.py 2014-12-23 16:59:05 +
> +++ mainpage/templatetags/wl_markdown
I don't really know how to deal with the map file being saved twice without
putting a lot of effort into it. I think it is not worth to investigate
further. From my side this is ready for review and merge.
--
https://code.launchpad.net/~widelands-dev/widelands-website/map-upload/+merge/227458
Yo
When Widelands is installed from the daily PPA the executables are also added
to PATH so it is not necessary to provide the exact directory. This means it
works without changes.
--
https://code.launchpad.net/~widelands-dev/widelands-website/map-upload/+merge/227458
Your team Widelands Developers
> Any idea why this is not showing a diff?
I got this mail form launchpad:
> Launchpad encountered an internal error during the following operation:
> generating the diff for a merge proposal. It was logged with id
> OOPS-3da48092fc23f6e12528857791597bea. Sorry for the inconvenience.
Does not
Shevonar has proposed merging lp:~widelands-dev/widelands-website/map-upload
into lp:widelands-website.
Requested reviews:
SirVer (sirver)
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands-website/map-upload/+merge/227458
This branch contains a first working draft of
Review: Approve
I had the time to review your changes. Did not notice anything obvious except
that you did not use Doxygen comment style but I know you are not very fond of
it ;)
--
https://code.launchpad.net/~widelands-dev/widelands/00_notification_framework/+merge/226975
Your team Widelands D
Review: Approve
Does compile on Ubuntu 14.04 with Clang 3.5 and GCC 4.8.2.
I also reviewed all your further changes and did not find anything to
criticize. The only thing is, I think we should not let the different libraries
get to small. A single header file in its own library is a little bit
Some more verbose output does not really help. The complete linker call is:
/usr/lib/gcc/x86_64-linux-gnu/4.8/collect2 --sysroot=/ --build-id
--eh-frame-hdr -m elf_x86_64 --hash-style=gnu --as-needed -export-dynamic
-dynamic-linker /lib64/ld-linux-x86-64.so.2 -z relro -o test_economy
/usr/lib/g
Since you cannot reproduce my issues I further investigated myself. I can fix
the widelands link error by adding widelands_ball_of mud as a dependency to
network. However, this is a cyclic dependency in the libraries. Therefore I had
to disable GLOBAL_DEPENDS_NO_CYCLES. IMHO the multi pass linki
Do the '# TODO(sirver): Has more dependencies than are listed here.' comments
mean that these dependencies are actually missing, meaning that it cannot
compile, or are they just not explicitly listed? In the first case my compile
problems are obviously also known.
--
https://code.launchpad.net/
Review: Needs Fixing
I just reviewed these changes. Nice to see to get some structure in the
Widelands source and build system! But there are still problems (on Ubuntu
14.04 with GCC 4.8):
The test_economy misses a reference to SDL_UnlockSurface when linking. The
dependency for SDL is missing
One last thing: The diff against target in this merge request says 'Text
conflict in src/graphic/animation.cc'. So won't be a clean merge.
--
https://code.launchpad.net/~widelands-dev/widelands/one_world/+merge/222708
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widel
Review: Approve
I just took the time to understand and simplify the
ResourceDescription::get_editor_pic method. The old behavior to fallback to a
resource pic with a negative upper limit was removed since it was not used
anyway. If you really don't want it in this commit you are free to revert
I changed some formatting issues and small obvious errors. I also changed
documentation to doxygen comment style so it gets picked up.
Here are the remaining questions:
- Why did you hard code the compiler in compile.sh
- Why are critter sound_effect split in directory and name and not just a pat
Shevonar has proposed merging lp:~widelands-dev/widelands/remove-double into
lp:widelands.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #756433 in widelands: "Remove --double option."
https://bugs.launchpad.net/widelands/+bug/756433
For more de
Thanks for your feedback SirVer, I changed it.
Thanks you Tino for testing on Windows.
I'll merge this now.
--
https://code.launchpad.net/~widelands-dev/widelands/fix-random-tribe/+merge/214451
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/fix-random-tribe.
Shevonar has proposed merging lp:~widelands-dev/widelands/fix-random-tribe into
lp:widelands.
Requested reviews:
Tino (tino79)
Related bugs:
Bug #1089731 in widelands: "random tribe always gives Empire"
https://bugs.launchpad.net/widelands/+bug/1089731
Bug #1302635 in wideland
Hopefully I got everything as you wish :D Don't hesitate to give directions for
further improvements because I don't really know how clean C++ code has to look
like.
--
https://code.launchpad.net/~widelands-dev/widelands/moved-classes/+merge/212302
Your team Widelands Developers is requested to
Shevonar has proposed merging lp:~widelands-dev/widelands/moved-classes into
lp:widelands.
Requested reviews:
Widelands Developers (widelands-dev)
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/moved-classes/+merge/212302
Moved classes from wlapplication.cc to own
Review: Needs Fixing
I just noticed a little misinterpretation: case SDLK_KP_ENTER is actually not
an empty switch case. It just falls through to case SDLK_RETURN to handle the
Enter key on the keypad the same way as the Return key. A case is only
redundant if it falls through to the default ca
Shevonar has proposed merging lp:~shevonar/widelands-website/bug-fixes into
lp:widelands-website.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1193837 in Widelands Website: "Forum text exceed borders"
https://bugs.launchpad.net/widelands-website/+b
Shevonar has proposed merging
lp:~widelands-dev/widelands-website/map-uploader-fixes into
lp:widelands-website.
Requested reviews:
SirVer (sirver)
Related bugs:
Bug #1092692 in Widelands Website: ""Pentagon" map download file name
incorrect"
https://bugs.launchpad.n
Shevonar has proposed merging lp:~shevonar/widelands-website/bug1202301 into
lp:widelands-website.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1202301 in Widelands Website: "Some maps not accepted by Map upload"
https://bugs.launchpad.net/widelan
Shevonar has proposed merging
lp:~shevonar/widelands-website/fix-missing-revisions into lp:widelands-website.
Requested reviews:
Widelands Developers (widelands-dev)
For more details, see:
https://code.launchpad.net/~shevonar/widelands-website/fix-missing-revisions/+merge/168897
This should
it yet but a migration guide can be found here [2]
[1] https://github.com/jtauber/django-notification/issues/60
[2]
https://docs.djangoproject.com/en/dev/topics/i18n/timezones/#time-zones-migration-guide
--
https://code.launchpad.net/~shevonar/widelands-website/django1.4/+merge/143219
Your team
Shevonar has proposed merging lp:~shevonar/widelands-website/django1.4 into
lp:widelands-website.
Requested reviews:
Widelands Developers (widelands-dev)
For more details, see:
https://code.launchpad.net/~shevonar/widelands-website/django1.4/+merge/143219
I updated Django and all other
I did the cleanup stuff now. So I'd say this branch is ready for testing on
alpha.widelands.com. The changes for the documentation is in the Apache
configuration only. I have to discuss some things with Sirver for the final
migration. Until now you can see the current state at http://wl.shevonar
What is missing IMHO (and I worked on that branch ;):
- Ware sorting in Encyclopedia: I can still do it later and it also requires
changes on Widelands code.
- The documentation has changed, I think we should put the new documentation in
a folder on the webserver and change the link (Sirver has t
Shevonar has proposed merging lp:~shevonar/widelands/opengl into lp:widelands.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #594686 in widelands: "Roads not rendered correctly in opengl mode on
some systems"
https://bugs.launchpad.net/widelands/+
Shevonar has proposed merging lp:~shevonar/widelands/nsis-cmake into
lp:widelands.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #543442 in widelands: "Replace InnoSetup script with CPack functionality"
https://bugs.launchpad.net/widelands/+bug/543442
Shevonar has proposed merging
lp:~shevonar/widelands/feature-random-tribe-and-AI into lp:widelands.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #722757 in widelands: "Play as a random tribe"
https://bugs.launchpad.net/widelands/+bug/722757
Bug
Shevonar has proposed merging lp:~shevonar/widelands/fix-cmake-codecheck into
lp:widelands.
Requested reviews:
Widelands Developers (widelands-dev)
For more details, see:
https://code.launchpad.net/~shevonar/widelands/fix-cmake-codecheck/+merge/76662
This solved my codecheck problem :) For
a minimal change of code (during initialization etc.) and network
synchronization. To show that a random tribe/ai is chosen the bool is necessary.
--
https://code.launchpad.net/~shevonar/widelands/feature-random-tribe-and-AI/+merge/75897
Your team Widelands Developers is requested to review the
Shevonar has proposed merging
lp:~shevonar/widelands/feature-random-tribe-and-AI into lp:widelands.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #722757 in widelands: "Play as a random tribe"
https://bugs.launchpad.net/widelands/+bug/722757
Bug
Shevonar has proposed merging lp:~shevonar/widelands/fix-ware-positioning into
lp:widelands.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #837239 in widelands: "Impossible to find suitable wares position for
carriers and beast of burdon alike.&quo
35 matches
Mail list logo
