Review: Approve
I can hardly comment on this. It seems very well crafted, but I have no
knowlegde about debian to say if this is alright for them. I suggest pining the
debian maintainer of widelands to have a look over this.
I'll go ahead and approve since this definitively looks like progress
Review: Needs Fixing
Just a nit: Please change:
if (not msgs . back() . sender . empty())
116 + // Alert me!
117 + play_new_chat_message();
to
if (!msgs.back().sender.empty()) {
// Alert me!
play_new_chat_message();
}
and I do not un
Review: Approve
again, I have no domain knowledge, so take my opinion with a grain of salt
--
https://code.launchpad.net/~hjd/widelands/debian-b18/+merge/208464
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/debian.
___
The PATH_MAX is the only patch that makes sense to me. The others look hacky to
me.
--
https://code.launchpad.net/~hjd/widelands/debian-b18/+merge/208464
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/debian.
___
Ma
We can apply the PATH_MAX patch, but I believe that it will only do something
on really old or broken systems.
--
https://code.launchpad.net/~hjd/widelands/debian-b18/+merge/208464
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/debian.
___
Feel free to push a non-compiling version that does not contain any merge
conflicts any more. I'll look into this.
as for the compiler warnings: you need gcc 4.6 or later (or clang 3) to compile
widelands since a c++11 transition I pushed to trunk recently. It made troubles
for lots of people,
I fixed the compile errors in trunk and ran the testsuite. So it seems like
everything is working fine now.
Are you able to compile trunk again? You'll need gcc 4.7 or later.
--
https://code.launchpad.net/~widelands-dev/widelands/gci18nfixes/+merge/192288
Your team Widelands Developers is reques
Mmh.. but it seems like the merge conflicts are not gone. If I do a
bzr merge lp:widelands
I still get tons of merge conflicts. We need to resolve those, otherwise we
will not be able to merge on wed. Can you have another look?
--
https://code.launchpad.net/~widelands-dev/widelands/gci18nfixe
just fyi: We did some more hacking on properly merging this branch yesterday.
The result can be found in lp:~widelands-dev/widelands/i18n - that is the
branch that we are going to merge on wednesday.
This merge request is therefore kinda outdated.
--
https://code.launchpad.net/~widelands-dev/w
Review: Approve
> - upper or lowercase needed here?
The strings have been changed to be titelized and unfortunately we depend on
them being the same. So uppercase should be correct.
> /utils/test/test_lua-xgettext.py
the print line and the comments (or non comments) are fine either way.
> Hel
We merged the i18n branch - so this merge request is superseeded by this merge
and therefore rejected.
--
https://code.launchpad.net/~widelands-dev/widelands/gci18nfixes/+merge/192288
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/gci18nfi
The proposal to merge lp:~widelands-dev/widelands/gci18nfixes into lp:widelands
has been updated.
Status: Needs review => Rejected
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/gci18nfixes/+merge/192288
--
https://code.launchpad.net/~widelands-dev/widelands/gci1
Review: Approve
lgtm.
--
https://code.launchpad.net/~widelands-dev/widelands/handle_tab/+merge/209406
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/handle_tab.
___
Mailing list: https://launchpad.net/~widelands-de
Review: Approve
shipit!
--
https://code.launchpad.net/~hjd/widelands/debian-disable-patch/+merge/210030
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/debian.
___
Mailing list: https://launchpad.net/~widelands-dev
P
I think so. Merged.
--
https://code.launchpad.net/~qcumber-some/widelands/bug1167242/+merge/21
Your team Widelands Developers is requested to review the proposed merge of
lp:~qcumber-some/widelands/bug1167242 into lp:widelands.
___
Mailing list: ht
Martin, your expertise is needed here. Is this something that should go
upstream?
--
https://code.launchpad.net/~qcumber-some/widelands/bug1287241/+merge/209091
Your team Widelands Developers is requested to review the proposed merge of
lp:~qcumber-some/widelands/bug1287241 into lp:~widelands-de
Thanks martin.
Jens, could you apply this patch then?
http://anonscm.debian.org/gitweb/?p=pkg-games/widelands.git;a=blobdiff;f=debian/rules;h=2affd4374e33bb967d8a7f355c431f271aee3efa;hp=64beaa47405397d92383bd6348b5e62a36ae1d82;hb=788f597602443038c0702fddb9fa345bfcf94484;hpb=a4e020f028c5d852d4c207
Could it be that we are not allowed to use more than one core on the build
servers? After all, canonical has to pay for them somehow.
--
https://code.launchpad.net/~qcumber-some/widelands/bug1287241/+merge/209091
Your team Widelands Developers is requested to review the proposed merge of
lp:~qcu
pretty hacky solution, but workable for now. We have a meddle between engine
and data files in more places.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-999262/+merge/210166
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-999262.
Review: Approve
looks good to me. I added one revision adressing all the FIXMEs. hjd, if you
are happy with my changes we can merge this imho.
--
https://code.launchpad.net/~widelands-dev/widelands/remove-record/+merge/210496
Your team Widelands Developers is subscribed to branch
lp:~widelands-
Review: Approve
Looks good to me, but GunChleoc is mistress of strings imho.
--
https://code.launchpad.net/~widelands-dev/widelands/fix-strings/+merge/210465
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/fix-strings.
Review: Approve
Looked over the FIXMEs and the merge request in general. LGTM. Some comments on
the FIXMEs
The first was the problem that name was a plain old c string (i.e. a pointer to
an array of characters). Constructing a c++ std::string out of it gives you
access to more utility methods.
load_finish() seems to be still needed - some derived classes override it.
--
https://code.launchpad.net/~widelands-dev/widelands/remove-compatibility-wares/+merge/210885
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/remove-compatibility-wares.
__
Review: Needs Fixing
I do not agree to this design. Especially since it needs new data files.
I suggest using code I added in
https://code.launchpad.net/~widelands-dev/widelands/spritemaps/+merge/211225
which will be merged soonish (hopefully). I suggest using
animation.representative_image to
I am confused about this merge request compared to
https://code.launchpad.net/~widelands-dev/widelands/debian-merged-b18-3/+merge/210502
? Can somebody merge one of them or both so that my confusion goes away?
--
https://code.launchpad.net/~qcumber-some/widelands/bug1287241/+merge/209091
Your te
I am confused about this merge request compared to
https://code.launchpad.net/~qcumber-some/widelands/bug1287241/+merge/209091 ?
Can somebody merge one of them or both so that my confusion goes away?
--
https://code.launchpad.net/~widelands-dev/widelands/debian-merged-b18-3/+merge/210502
Your
I think resizing with keeping the aspect ratio is the way to go. The menu
pictures we have right now have been created in the same way. Basically
something like:
double ratio = 32. / std::max(image_w, image_h);
resize(image_w * ratio, image_h * ratio);
--
https://code.launchpad.net/~widelands-d
It did Yesterday. I think launchpad is failing here. I guess the review has to
be offline.
> Am 17.03.2014 um 16:03 schrieb Jens Beyer :
>
> Looks like this branch does not merge into trunk without conflicts around the
> Barbarian ferner?
> --
> https://code.launchpad.net/~widelands-dev/widel
Lgtm.
> Am 17.03.2014 um 15:25 schrieb Tino :
>
> Tino has proposed merging lp:~widelands-dev/widelands/tooltips_fixes into
> lp:widelands.
>
> Requested reviews:
> Widelands Developers (widelands-dev)
>
> For more details, see:
> https://code.launchpad.net/~widelands-dev/widelands/tooltips_
I went ahead and merged this now, because this branch seems weird. I'd
appreciate code comments still - it is probably easiest to look at revision
r6880 in trunk to see all the changes that were introduced.
--
https://code.launchpad.net/~widelands-dev/widelands/spritemaps/+merge/211225
Your te
Review: Approve
lgtm.
--
https://code.launchpad.net/~widelands-dev/widelands/debian-merged-b18-3/+merge/210502
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/debian.
___
Mailing list: https://launchpad.net/~wideland
Okay, so I set this one to rejected, and somebody gotta merge the other one.
Jens, if you feel something in this branch is not in the other, you should
speak up now :)
--
https://code.launchpad.net/~qcumber-some/widelands/bug1287241/+merge/209091
Your team Widelands Developers is requested to re
The proposal to merge lp:~qcumber-some/widelands/bug1287241 into
lp:~widelands-dev/widelands/debian has been updated.
Status: Needs review => Rejected
For more details, see:
https://code.launchpad.net/~qcumber-some/widelands/bug1287241/+merge/209091
--
https://code.launchpad.net/~qcumber-so
looks good and works great. I changed the code to use get_animation("idle")
instead of main_animation() - which should probably be removed anyways. I also
dropped a NOCOM(#qcs) for you.
I would also suggest doing a side by side comparison between the new menu and
the old to see if quality has d
const Image& anim_frame =
g_gr->animations().get_animation(descr.get_animation("idle"))
.representative_image(RGBColor(0, 0, 0));
well, here was the problem. You construct an image with playercolor black,
always. I changed it to use the true player color.
While doing that I also checked the di
And there is a bilinear interpolation ready to be copy&pasted:
http://www.scratchapixel.com/lessons/3d-advanced-lessons/interpolation/bilinear-interpolation/
That is maybe already good enough.
--
https://code.launchpad.net/~widelands-dev/widelands/buildicon_playercolors/+merge/211401
Your team W
I looked around the SDLgfx source code real quick and saw that it provides a
shrinking method that gives better quality then just using zoom to shrink. I
added a quick hack to shrink the image to ballpark the size, then zoomSurface
to get it correct. It still looks worse than menu.png, but much
I support this refactoring task and raise my pint for you!!
- While you do the refactorings, could you get rid of the abbreviations too,
please
e.g. sp_gamecontroller.[cc|h] -> single_player_game_controller.[cc|h].
- Add 'override' to derived classes.
- Extra points if you move method definitions
looks good to me. I merged and deployed on the server - thanks for your work on
this!
--
https://code.launchpad.net/~widelands-dev/widelands-metaserver/ircbridge/+merge/203277
Your team Widelands Developers is subscribed to branch lp:widelands-metaserver.
ping?
--
https://code.launchpad.net/~widelands-dev/widelands/buildicon_playercolors/+merge/211401
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/buildicon_playercolors into lp:widelands.
___
Mail
Review: Approve
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml
more often than not this is the style I am referring to. It is pretty well
thought out.
lgtm! feel free to merge.
--
https://code.launchpad.net/~widelands-dev/widelands/moved-classes/+merge/212302
Your team Wideland
Review: Approve
--
https://code.launchpad.net/~widelands-dev/widelands/yet-some-more-tiny-fixes/+merge/213382
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/yet-some-more-tiny-fixes.
___
Mailing list: https://launc
Merged.
--
https://code.launchpad.net/~widelands-dev/widelands/buildicon_playercolors/+merge/211401
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/buildicon_playercolors into lp:widelands.
___
M
Should there not be a checkbox in the chat somewhere to disable forwarding
messages from IRC which would also disable the sound?
I think there is currently no other way to identify IRC messages then by
string. Feel free to add a TODO - I guess the protocol needs changing anyways
for remote host
The proposal to merge lp:~widelands-dev/widelands/split-assert into
lp:widelands has been updated.
Status: Needs review => Merged
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/split-assert/+merge/214432
--
https://code.launchpad.net/~widelands-dev/widelands/spli
My bad - too much python. I fixed it in trunk manually without merging this
one.
--
https://code.launchpad.net/~widelands-dev/widelands/split-assert/+merge/214432
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/split-assert into lp:widelan
Review: Approve
Just a minor nit:
- Move the factory methods at the top of the class and document them.
Otherwise LGTM.
--
https://code.launchpad.net/~widelands-dev/widelands/scriptconsole/+merge/214439
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/scriptco
- Move the factory methods at the top of the class and document them.
in the header I mean :)
--
https://code.launchpad.net/~widelands-dev/widelands/scriptconsole/+merge/214439
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/scriptconsole.
Review: Approve
> Is the IRC bridge now frozen or is it still evolving?
That depends. Tino wrote it and I have no idea if he has plans on continue
working on it. I am too loaded to improve it tough I find the project very
intriguing. It is not "feature complete" as there is no design doc for it
Review: Needs Fixing
A nit:
- Pull the srand initialization into wlapplication (i.e. our 'main' function
equivalent) and only do it once. Should we ever want to inject a seed (for
testing) it should only be done in one place and this should have the same
effect in all your call sites.
--
https
The diff is hard to read - is the consumed ware/produced ware still the same
given that the mine is always fully filled?
I also suggest fixing all mines at once.
--
https://code.launchpad.net/~widelands-dev/widelands/feature-1478-stone-mine-streamline/+merge/214822
Your team Widelands Developers
Review: Approve
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1300724/+merge/214666
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1300724.
___
Mailing list: https://launchpad.net/~widelands-dev
Po
Review: Approve
lgtm.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1290070/+merge/215005
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1290070.
___
Mailing list: https://launchpad.net/~widelands-d
Updating the database though always feels like surgery on the open heart
without backup. That is the main reason why we did not update - it is just so
risky and time consuming. Especially with Python where you never know if your
code is actually broken till it breaks.
I agree to your reasoning
That is of course what we do. But still: you have a dump file, then you update
all the software and hope that they really play as well together as in the
tests. Then you run the migration scripts that usually fail somewhere halfway
through - you have a database in a weird state then. You pick up
lgtm. just a nit:
+ rt("image=tribes/atlanteans/".. f.immovable.name .."/menu.png",
Add a todo here that this should representative_image as soon as we wrap it.
Relying on menu.png being there is fragile, especially since there was already
talk to remove menu.png pictures as they are not us
Review: Approve
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1298309/+merge/215707
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1298309.
___
Mailing list: https://launchpad.net/~widelands-dev
Po
I do not quite understand the idea behind this request. Empty mines are never
completely empty, right? So there is always the chance that something is
produced and that will level the worker. Is that not enough?
--
https://code.launchpad.net/~widelands-dev/widelands/feature-1478-stone-mine-strea
I remeber this discussion, but did we not agree that streamlining the mines to
only dig for resources once is enough in these cases?
--
https://code.launchpad.net/~widelands-dev/widelands/feature-1478-stone-mine-streamline/+merge/215594
Your team Widelands Developers is requested to review the pr
Review: Approve
Code looks good to me. I will also change the globbing though.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1304638-firstfix/+merge/215932
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1304638-firstfix.
_
Review: Approve
Thanks for your clarifications. I thought we could fix that by consuming twice
the wares, reduce the amount of coal by twice and produce twice the wares. But
that has other implications (i.e. you need to have twice the wine before
anything is produced). So I think your approach
Review: Approve
maybe you can also get rid of some header? otherwise lgtm.
--
https://code.launchpad.net/~widelands-dev/widelands/remove-double/+merge/216066
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/remove-double.
___
Review: Approve
seems straightforward to me.
--
https://code.launchpad.net/~hjd/widelands/add-regex-dependency/+merge/216268
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/debian.
___
Mailing list: https://launchpad
Review: Approve
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1292828-cleanup/+merge/216705
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1292828-cleanup.
___
Mailing list: https://launchpad.net/~
Review: Approve
I looked through this and I wondered why you did not also delete the patch
file. Unused files in a repository are usually a bad idea, since they are
forgotten over time. If somebody needs it back in the future, they can
reverse-cherrypick your commit.
Otherwise lgtm.
--
https:
I Approve of the reasoning and the change.
> Am 26.04.2014 um 18:10 schrieb Hans Joachim Desserud :
>
> Hans Joachim Desserud has proposed merging
> lp:~hjd/widelands/remove-all-patches into lp:~widelands-dev/widelands/debian.
>
> Requested reviews:
> Widelands Developers (widelands-dev)
>
>
I see some merge conflicts in this diff down there. Might want to fix those
before this goes to trunk.
How do you want to move forward with this? From experience I can tell you that
at most 2 or 3 people will test your changes when they are in a separate
branch, but you will get a lot of feedb
Review: Needs Fixing
A bunch of small nits.
Diff comments:
> === modified file 'src/editor/tools/editor_info_tool.cc'
> --- src/editor/tools/editor_info_tool.cc 2014-03-09 10:28:39 +
> +++ src/editor/tools/editor_info_tool.cc 2014-05-29 17:16:25 +
> @@ -49,16 +49,11 @@
>
Review: Needs Fixing
some suggestions.
Diff comments:
> === modified file 'doc/sphinx/source/tutorial.rst'
> --- doc/sphinx/source/tutorial.rst2013-04-03 09:06:34 +
> +++ doc/sphinx/source/tutorial.rst2014-05-29 10:48:20 +
> @@ -269,4 +269,62 @@
> coroutines share the same envir
Review: Needs Fixing
some comments
Diff comments:
> === modified file 'maps/MP Scenarios/Island
> Hopping.wmf/scripting/multiplayer_init.lua'
> --- maps/MP Scenarios/Island Hopping.wmf/scripting/multiplayer_init.lua
> 2014-03-25 06:18:48 +
> +++ maps/MP Scenarios/Island Hopping.wmf/s
This is a lot to review and I think I will have a bunch of comments. Should I
do it here or rather in the code?
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1074353/+merge/221095
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands
Review: Approve
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1293158/+merge/221434
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1293158.
___
Mailing list: https://launchpad.net/~widelands-dev
Po
Review: Approve
the inline comment is rather cumbersome with saving
Diff comments:
> === modified file 'src/editor/tools/editor_info_tool.cc'
> --- src/editor/tools/editor_info_tool.cc 2014-03-09 10:28:39 +
> +++ src/editor/tools/editor_info_tool.cc 2014-05-29 17:16:25 +
>
Review: Approve
Diff comments:
> === modified file 'doc/sphinx/source/tutorial.rst'
> --- doc/sphinx/source/tutorial.rst2013-04-03 09:06:34 +
> +++ doc/sphinx/source/tutorial.rst2014-06-05 10:33:32 +
> @@ -5,7 +5,7 @@
>
> This section describes how to get a map ready for scri
Review: Approve
Diff comments:
> === modified file 'doc/sphinx/source/tutorial.rst'
> --- doc/sphinx/source/tutorial.rst2013-04-03 09:06:34 +
> +++ doc/sphinx/source/tutorial.rst2014-05-29 10:48:20 +
> @@ -269,4 +269,62 @@
> coroutines share the same environment and can therefo
Diff comments:
> === modified file 'maps/MP Scenarios/Island
> Hopping.wmf/scripting/multiplayer_init.lua'
> --- maps/MP Scenarios/Island Hopping.wmf/scripting/multiplayer_init.lua
> 2014-03-25 06:18:48 +
> +++ maps/MP Scenarios/Island Hopping.wmf/scripting/multiplayer_init.lua
Review: Approve
Diff comments:
> === modified file 'maps/MP Scenarios/Island
> Hopping.wmf/scripting/multiplayer_init.lua'
> --- maps/MP Scenarios/Island Hopping.wmf/scripting/multiplayer_init.lua
> 2014-03-25 06:18:48 +
> +++ maps/MP Scenarios/Island Hopping.wmf/scripting/multiplay
> 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
The proposal to merge lp:~widelands-dev/widelands/one_world into lp:widelands
has been updated.
Description changed to:
This merges all worlds into one, converts its configuration to Lua, adds the
compatibility layer to map loading needed for this.
This also regresses some issues:
- Bug 13286
Review: Needs Information
I'd rather not do the juggling with the translations in this branch/merge
request. I did the test merge of all de.po of the old worlds into a new one in
a new branch and pushed for inspection:
https://code.launchpad.net/~widelands-dev/widelands/one_world_translations
Gun, let me know how you want to do the merge - i.e. announcing the merge or
what. There are not that many strings in the world though, so letting the fuzzy
strings die is probably alright.
--
https://code.launchpad.net/~widelands-dev/widelands/one_world/+merge/222708
Your team Widelands Develop
I added a bunch of more commits. I think this should be merged ASAP. Are there
still any outstanding things you want to look at (except for the one code
review TODO).
--
https://code.launchpad.net/~widelands-dev/widelands/cmake-reworked/+merge/222455
Your team Widelands Developers is requested t
I really want to merge this ASAP. I hoped to get a code review - but it seems
nobody has the time right now :(.
So grab the archive whenever you can and inform the forum.
--
https://code.launchpad.net/~widelands-dev/widelands/one_world/+merge/222708
Your team Widelands Developers is subscribed
I addressed the final code review TODO. This is ready for merging imho.
Could somebody test on Linux and Windows? And if anyone wants to make a code
review too - that would be great :).
--
https://code.launchpad.net/~widelands-dev/widelands/cmake-reworked/+merge/222455
Your team Widelands Develo
Cool! I think it is easiest to just litter the code with NOCOMMIT(#sirver) or
something greppable. I can then take a look at those things.
--
https://code.launchpad.net/~widelands-dev/widelands/one_world/+merge/222708
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev
Inline comments are fine too, of course.
--
https://code.launchpad.net/~widelands-dev/widelands/one_world/+merge/222708
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/one_world.
___
Mailing list: https://launchpad.ne
Thanks for the review! It is very much appreciated. You also caught some bugs
that I have missed. Code reviews are such an awesome thing! nearly as good as
pair programming.
> - Why did you hard code the compiler in compile.sh
good catch. gun did that and probably submitted the change by accide
Thanks for taking care of this and for the review in general. I'll go ahead and
merge then. I also merge the translations as Gun proposed and as we already
tested basically works.
--
https://code.launchpad.net/~widelands-dev/widelands/one_world/+merge/222708
Your team Widelands Developers is sub
On 18.06.2014, at 13:21, Tibor Bamhor wrote:
> Hi,
>
> this is programming-related question, I can not figure it out (no wonder
> with my experiences):
>
> I need to count fields with fishes (currently no such feature in AI), I
> somehow came to this:
>
> if (field.water_nearby_ >0){
> map.fi
there is a lot of types of animals…
all of them are editable and huntable. So what?
>
>
>
> 2014-06-18 13:36 GMT+02:00 SirVer :
>
>>
>> On 18.06.2014, at 13:21, Tibor Bamhor wrote:
>>
>>> Hi,
>>>
>>> this is programmin
Tino, your issues should be fixed now.
--
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.
___
Thanks for reviewing, Shevonar! I am very grateful that you took up reviewing
changes.
> It seems that the WL_SRC set contains most of the source files twice: once
> with indentation, once without.
Every file was only there once, but the indent was not consistent. Fixed.
[minizip]
I much rathe
> However, this is a cyclic dependency in the libraries. Therefore I had to
> disable GLOBAL_DEPENDS_NO_CYCLES.
I disalbed this now. It would be a lie to set it anyways - we have cyclic
dependencies and though I wish we would not have them, setting this flag will
not make them go away. network
I think this is now good to go. Shevonar, can you test on Linux again? I'll
merge this tomorrow if nobody cries out right now :).
--
https://code.launchpad.net/~widelands-dev/widelands/cmake-reworked/+merge/222455
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands
[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 probl
Review: Approve
lgtm.
--
https://code.launchpad.net/~hjd/widelands/debian-dependencies/+merge/224032
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/debian.
___
Mailing list: https://launchpad.net/~widelands-dev
Post
Review: Approve
--
https://code.launchpad.net/~widelands-dev/widelands/silence_Wpedantic/+merge/224179
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/silence_Wpedantic.
___
Mailing list: https://launchpad.net/~wide
of course a similar approach can also taken for 2.8.7, though I expect more
wrapper macros then.
--
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.
___
SYSTEM will include the dir using -Isystem instead of -I which
will silent warnings. It is really, really useful when doing development, so it
should be kept for all cmake versions that support it. Suggested way forward:
1) lower required cmake version to 2.8.11
2) write a macro that wraps targ
SirVer has proposed merging
lp:~widelands-dev/widelands/kill_frontier_and_flag_styles into lp:widelands.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1332842 in widelands: "Remove support for different flag and frontier
'styles'."
https:
301 - 400 of 1693 matches
Mail list logo
