Review: Needs Fixing
Okay, done reviewing. I think this is nearly ready. I added a bunch of NOCOM,
let me know if you need help with any of them.
I see a bunch of followup tasks that result from this branch. I outline them
here, but they do not need to be done in this branch. If you do not want
This still needs numbers to be put in. The corresponding bug has suggestions. I
might be able to do it this week - not sure though.
--
https://code.launchpad.net/~widelands-dev/widelands/terrain_affinity/+merge/224045
Your team Widelands Developers is requested to review the proposed merge of
lp
should be fixed. Can you try again?
--
https://code.launchpad.net/~widelands-dev/widelands/richtext_testing/+merge/225733
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/richtext_testing.
___
Mailing list: https://lau
Thanks tino!
I went ahead and merged this now as it seems nobody wanted to review the code.
--
https://code.launchpad.net/~widelands-dev/widelands/terrain_affinity/+merge/224045
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/terrain_affinity.
one more try. I try now to initialize all pixels, maybe the differences now go
away.
--
https://code.launchpad.net/~widelands-dev/widelands/richtext_testing/+merge/225733
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/richtext_testing.
___
Scroll down in this diff view: You see a lot of (properties changed: -x to +x).
you marked files as executable that should not be. chmod -x those files and
commit them again.
or even better. create a new branch, move only the source files over and
propose for merging this instead - this one is
Thanks for looking into this again. I am worried that there is (also?) a real
bug lurking in the code and we are not properly updating the Alpha channel
somehow. I will add something that randomizes the initially created surfaces to
make sure that we are really updating everything.
--
https://c
First: FreeBSD is not a prority at all. I think it is not worth spending much
time on this. I believe that even Linux is not our main player population.
I think the clocale might clash with gettext/libintl on some systems. I am not
positive on that though. It is saver to include it using a #ifde
Review: Resubmit
One more test please. I found a machine were the tests did not pass and I
copied their wrong.png to correct.png and the tests still pass on my machine -
weirdly enough. This points me to some weirdness in how PNGs are saved or
loaded.
My guess is that the tests now pass on you
1) mark this merge request as abandoned and open a new one then, please.
2) This means you should have '#include "ai/defaultai.h"' as first include in
the file, before even including c/c++ libraries.
for the rest we will just have a look in the new branch again.
you can codecheck a single file w
After positive feedback from win32 and linux I merged this now. Thanks for the
help everybody.
--
https://code.launchpad.net/~widelands-dev/widelands/richtext_testing/+merge/225733
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/richtext_testing.
_
SirVer has proposed merging lp:~widelands-dev/widelands/map_information into
lp:widelands.
Requested reviews:
Widelands Developers (widelands-dev)
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/map_information/+merge/226572
Suggested submit message:
-
- Add a
Review: Needs Fixing
It annoys me telling you the same things again (like not including SDL_types.h
which I think I did two times before or removing the Default AI comment)
but I will do it one more time. This time I did it as inline comments, so that
it will not get lost in the review proc
Review: Approve
--
https://code.launchpad.net/~widelands-dev/widelands/world-stringfix/+merge/226426
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/world-stringfix.
___
Mailing list: https://launchpad.net/~wideland
Review: Approve
Oversight on my part. Thanks for catching this.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1339861/+merge/226570
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1339861.
___
Maili
the problem was that number of players is a uchar (i.e. 0 <= uchar <= 255).
Boost::format() likes to interpret this though as a 'single char'. I merged
this and static_cast() the value in question and all is good.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1293158/+merge/221434
Cool! Merged in r 7074.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1324137/+merge/221353
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1324137.
___
Mailing list: https://launchpad.net/~widelands-
what happens with this branch now, hjd?
--
https://code.launchpad.net/~widelands-dev/widelands/freebsd/+merge/225867
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/freebsd into lp:widelands.
___
Review: Needs Fixing
The branch contains conflicts. Renaming the building will break save game
compatibility, but I think that is not much of a problem.
Have you grepped for 'weaver' to catch really all occurrences?
--
https://code.launchpad.net/~widelands-dev/widelands/rename_goldweaver/+merge
Review: Approve
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1341112/+merge/226586
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1341112.
___
Mailing list: https://launchpad.net/~widelands-dev
Po
Review: Needs Fixing
Do you really want to do this? I ask because I think that seashells are a kind
of skeleton :P.
Also, the implications here are that all maps created between the one world
merge and this will be unloadable (and there have been at least 2 linked in the
forums already) withou
Review: Approve
Please revert catalogue updates in the future before sending a merge request.
It makes reviewing uglier and it is better to update translations and
catalogues in trunk anyways.
Rest of the changes lgtm and I merged them right away.
--
https://code.launchpad.net/~widelands-dev/
Review: Approve
> I have not been able to run the codecheck target at all.
should be as simple as cd build && make codecheck. What is the problem?
I changed the codecheck rule and remove one that is now redundant while
merging. Otherwise LGTM.
Thanks for doing these cleanups :)
Diff commen
disregard the inline comments, I wrote them before deciding that fixing would
be faster.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1330599/+merge/226607
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1330599.
Review: Approve
a) that is fine.
b) Right, that was in late initialization. Sorry for missing that.
lgtm. I went ahead and merged.
Thanks for the contribution!
--
https://code.launchpad.net/~widelands-dev/widelands/tibor-ai4/+merge/226029
Your team Widelands Developers is subscribed to branc
Review: Resubmit
> Could you give some more details on how you see this being handled on the
> website-side, and what will be needed there?
The code is needed on upload of a map to wl.widelands.org/maps. It has to
verify that a map is valid, if it is post one_world (i.e. only working with >=
d
SirVer has proposed merging lp:~widelands-dev/widelands/bug-1332627 into
lp:widelands.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1332627 in widelands: "Remove use of boost::noncopyable"
https://bugs.launchpad.net/widelands/+bug/1332627
For mo
Review: Needs Fixing
Great work!!!
- I think we should be consistent about where to place these macros.
1) Google style is to have them as the last thing in the class and always in a
private statement. I like this because I am used to it and because it is
already 'out there'.
2) One could a
Review: Needs Fixing
Diff comments:
> === modified file 'src/base/i18n.cc'
> --- src/base/i18n.cc 2014-06-21 15:17:04 +
> +++ src/base/i18n.cc 2014-07-14 12:52:01 +
> @@ -19,6 +19,10 @@
>
> #include "base/i18n.h"
>
> +#ifdef __FreeBSD__
> +# include
> +#endif
> +
> #include
>
Review: Approve
--
https://code.launchpad.net/~widelands-dev/widelands/rename_goldweaver/+merge/226580
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/rename_goldweaver.
___
Mailing list: https://launchpad.net/~wide
TypeName() = default; should be here
ITransportCostCalculator() = default
And I was wrong in my first comment. It is needed when the copy constructor is
deleted.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1332627/+merge/226628
Your team Widelands Developers is subscribed to bran
No, changing the descname is fine and can be done without issues. I think this
is preferable to Teppos suggestion actually.
--
https://code.launchpad.net/~widelands-dev/widelands/fix_world_names/+merge/226592
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/fix_
Review: Approve
hardly anything to complain about in this branch :P.
--
https://code.launchpad.net/~widelands-dev/widelands/skeletons2seashells/+merge/226725
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/skeletons2seashells.
_
Awesome! Merged. I tweaked the codecheck rule and fixed the remaining
boost::noncopyable. The linking error probably came from not fixing the
dependencies in the CMakeLists.txt files. make codecheck lists those.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1332627/+merge/226628
Yo
On second thought, that cannot really explain linker errors. If you still have
them after a make clean please open a bug report and we'll investigate.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1332627/+merge/226628
Your team Widelands Developers is subscribed to branch
lp:~widel
Review: Approve
--
https://code.launchpad.net/~widelands-dev/widelands/mapobject_cleanup_misc/+merge/226756
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/mapobject_cleanup_misc.
___
Mailing list: https://launchpad
Review: Approve
Applied after fixing a small merge conflict due to recent merges.
--
https://code.launchpad.net/~widelands-dev/widelands/mapobject_cleanup_building/+merge/226755
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/mapobject_cleanup_building.
__
Review: Approve
--
https://code.launchpad.net/~widelands-dev/widelands/mapobject_cleanup_soldier/+merge/226754
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/mapobject_cleanup_soldier.
___
Mailing list: https://lau
Review: Approve
one question. But I went ahead and merged already.
Diff comments:
> === modified file 'src/economy/idleworkersupply.cc'
> --- src/economy/idleworkersupply.cc 2014-06-01 18:00:48 +
> +++ src/economy/idleworkersupply.cc 2014-07-14 20:49:30 +
> @@ -79,7 +79,7 @@
> void
The proposal to merge lp:~hjd/widelands/add_flag into lp:widelands has been
updated.
Status: Needs review => Merged
For more details, see:
https://code.launchpad.net/~hjd/widelands/add_flag/+merge/226741
--
https://code.launchpad.net/~hjd/widelands/add_flag/+merge/226741
Your team Widelands
Tino did this in trunk this morning too.
--
https://code.launchpad.net/~hjd/widelands/add_flag/+merge/226741
Your team Widelands Developers is requested to review the proposed merge of
lp:~hjd/widelands/add_flag into lp:widelands.
___
Mailing list: htt
Review: Approve
Cool, that is how I expected it to look. I merged to trunk now.
--
https://code.launchpad.net/~widelands-dev/widelands/freebsd/+merge/225867
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/freebsd.
__
Review: Approve
I looked over this again and fixed some formatting issues, merged trunk, tested
real quick and merged.
Thanks for taking care of this.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1322473/+merge/221235
Your team Widelands Developers is subscribed to branch
lp:~wid
Anyone?
--
https://code.launchpad.net/~widelands-dev/widelands/map_information/+merge/226572
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/map_information.
___
Mailing list: https://launchpad.net/~widelands-dev
Post
unistd.h is a unix only header. So you have to wrap it in a #ifdef.
--
https://code.launchpad.net/~hjd/widelands/freebsd-unlink/+merge/226905
Your team Widelands Developers is requested to review the proposed merge of
lp:~hjd/widelands/freebsd-unlink into lp:widelands.
__
I adressed the remaining comments in 6926.
One question was how to get a object from c++ to Lua. There is a template
function to_lua<> for this. Usage is like this:
to_lua(L, new
L_WareDescription(tribe.get_ware_descr(ware_amount.first)));
This generates a L_WareDescription and pushes it at t
I found a couple of minor bugs and made the testsuite pass again. Great that
you wrote tests for all your methods! It made it really easy to find,
understand and fix bugs.
I merged this now. This is a great contribution and a big chunk of work! Thanks
for your persistence with this :). It great
Review: Approve
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1074353/+merge/221095
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1074353.
___
Mailing list: https://launchpad.net/~widelands-dev
Po
> Did not notice anything obvious except that you did not use Doxygen comment
> style but I know you are not very fond of it ;)
:( It is not bad will, I just strive for consistency and fall into the habit of
doing docs the way we do it at work. Also my editor is not configured to deal
with doxy
Review: Approve
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1342801/+merge/227162
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1342801.
___
Mailing list: https://launchpad.net/~widelands-dev
Po
Review: Needs Fixing
> This avoids having to get the tribes' Immovable_Descr through the Lua
> interface. If you think this is a problem, I can have a go at it though.
> Otherwise, ready for merging.
I do not think this is a problem. Wrapping those too might make sense in the
future anyways th
There is also a diff comment buried in my last comment.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1341081/+merge/227107
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1341081.
___
Mailing list: h
Review: Needs Fixing
Rename the lua files too for consistency - so that all are named the same.
Might need to grep() for the filenames to get all uses.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1342563/+merge/226981
Your team Widelands Developers is subscribed to branch
lp:~wid
I'll go ahead and merge this now.
--
https://code.launchpad.net/~widelands-dev/widelands/map_information/+merge/226572
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/map_information.
___
Mailing list: https://launch
Review: Approve
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1342563/+merge/226981
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1342563.
___
Mailing list: https://launchpad.net/~widelands-dev
Po
Review: Needs Fixing
Diff comments:
> === modified file 'campaigns/atl01.wmf/scripting/init.lua'
> --- campaigns/atl01.wmf/scripting/init.lua2014-07-17 15:26:32 +
> +++ campaigns/atl01.wmf/scripting/init.lua2014-07-19 09:00:26 +
> @@ -324,7 +324,7 @@
>if not f.immovable
Review: Needs Fixing
Diff comments:
> === modified file 'compile.sh'
> --- compile.sh2014-06-18 17:11:08 +
> +++ compile.sh2014-07-19 14:18:30 +
> @@ -26,15 +26,14 @@
> # TODO user interaction and functions for installation including a check
these TODOs all seem like
Review: Needs Fixing
I think lumberjacks should only send a message after ~10 failed attempts or so.
Otherwise they will send too often.
Rest of the comments inlined in the code in r7115.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-999262_part2/+merge/227348
Your team Widelands D
Ahh, save comment to quick again. That is a great change and fairly complex and
I just wanted to say that you came a long way as a coder in a very short time.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-999262_part2/+merge/227348
Your team Widelands Developers is subscribed to bran
The inline diff tools is pretty broken - all your comments are invisible now.
So I comment via Email instead.
> - link to somewhere on the wiki which explains manual building in more detail
> - fix indentation and creation of update.sh
Feel free to do indentation in the future right away. I revi
Review: Approve
if you take care of the couple of nits we discussed it looks good to me.
--
https://code.launchpad.net/~widelands-dev/widelands/trimming-compile/+merge/227390
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/trimming-compile.
___
This change would still be awesome to have in Widelands. Charly, you are not by
any chance taking things on again soon?
--
https://code.launchpad.net/~widelands-dev/widelands/storages_garrisons/+merge/178704
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/stora
Cool. I am working on getting rid of the old font renderer too in
https://code.launchpad.net/~widelands-dev/widelands/text_area_richtext . Maybe
we can join forces.
--
https://code.launchpad.net/~widelands-dev/widelands/storages_garrisons/+merge/178704
Your team Widelands Developers is subscrib
I added a bunch of NOCOM(#codereview). You do not implement some methods (see
the codereview comments). The code I pushed still doesn't compile, but it
should give you an idea of what is missing.
Also, your editor formats source code quite differently to the codebase, so
there are a lot of styl
> To make it work Widelands must be installed (make install) and the
> wl_map_info executable moved to the widelands-website root (next to
> manage.py).
I think we should be able to provide a path to a directory that contains all
widelands binaries. On the server this is conveniently
/usr/shar
Any idea why this is not showing a diff?
--
https://code.launchpad.net/~widelands-dev/widelands-website/map-upload/+merge/227458
Your team Widelands Developers is subscribed to branch lp:widelands-website.
___
Mailing list: https://launchpad.net/~widela
Yes, we are using tabs. Personally, I'd much rather switch to spaces and
reformat all code with clang-format, which would then be the definition of
'correct'. I got push back the last two times I tried to establish this though.
> The codecheck warnings are hidden behind the "check to see if code
Review: Approve
Looks good to me. I went ahead and merged. I'll probably tweak a bit while
deploying...
Thanks for taking care of that Shevonar!
--
https://code.launchpad.net/~widelands-dev/widelands-website/map-upload/+merge/227458
Your team Widelands Developers is subscribed to branch lp:wid
Review: Approve
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1322741/+merge/227626
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1322741.
___
Mailing list: https://launchpad.net/~widelands-dev
Po
Review: Needs Fixing
I changed a couple of nits:
- I moved the documentation of your new methods to the header (documentation in
.cc files makes less sense. I know that we did that in the past, but for the
future, documentation goes into the header).
- Added a disallow_copy_and_assign() to Pr
Review: Approve
--
https://code.launchpad.net/~widelands-dev/widelands/string_fix_headquarters/+merge/227543
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/string_fix_headquarters.
___
Mailing list: https://launchp
That's a lot of code. I need to make a chunk of time to review this.
My branch is about removing the legacy richtext renderer and using the new one
everywhere (i.e. especially in scenario texts).
--
https://code.launchpad.net/~widelands-dev/widelands/fh1/+merge/177228
Your team Widelands Develo
Review: Needs Fixing
1) Is strange, but makes sense. I do not fully understand why that is needed
now and wasn't before, but meh.
2) Dangerous assumption. The speed of a dynamic_cast() depends hugely on many
factors. String comparison is always slow. See
http://stackoverflow.com/questions/9778
Review: Approve
looks good.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-999262_part2/+merge/227348
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-999262_part2.
___
Mailing list: https://launchpad.
The aim should be to get this (and my branch) merged into trunk ASAP and then
branch again to tackle the remaining issues. Smaller branches are much easier
to work with anyways.
I try to look into this in more detail on the weekend.
--
https://code.launchpad.net/~widelands-dev/widelands/fh1/+me
I am very meh about the 'unknowns'. If somebody has a script, let's do it.
Otherwise we'll keep this as legacy.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1341674/+merge/227693
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands
Review: Needs Fixing
Couple of comments in the code. Only small things.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1341079/+merge/227441
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1341079.
__
Barbarians do not build houses. I understand that there is movement to unify
names, but please also keep the in mind that the three tribes should be
different and should have their own individuality.
aD, I added you as a reviewer since your a native speaker. Can you add anything
to the discussi
Review: Approve
I merged this now, but removed the TODO discussion around spritesheets. I think
it can be handled transparently - .menu can return a hash of an image in the
image cache. Since the renderer uses the image cache for looking up it's
images, this should be opaque for the renderer. T
Review: Approve
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1341674/+merge/227693
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1341674.
___
Mailing list: https://launchpad.net/~widelands-dev
Po
Review: Needs Fixing
Diff comments:
> === added file 'cmake/codecheck/rules/format_TODO_comments'
> --- cmake/codecheck/rules/format_TODO_comments1970-01-01 00:00:00
> +
> +++ cmake/codecheck/rules/format_TODO_comments2014-07-23 14:52:16
> +
> @@ -0,0 +1,21 @@
> +#!/us
I fixed pathfield.cc and removed container_iterate_const. wl_range is still
used, but as far as I can see only in places that can just use a for (const T&
blub : container) {} loop.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1203629/+merge/228203
Your team Widelands Developers is
Review: Needs Fixing
This needs trunk merging really bad.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1341081/+merge/227107
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1341081.
___
Mailing list
Review: Approve
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1343302/+merge/227432
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1343302.
___
Mailing list: https://launchpad.net/~widelands-dev
Po
Review: Approve
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1343302_codecheck/+merge/228357
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1343302_codecheck.
___
Mailing list: https://launchpad.n
Review: Approve
I go ahead and merge this now. I did review, but it is super hard to spot
mistakes in a refactoring like this. We will hear of bugs sooner or later.
The remaining wl_const use is tracked in bug 1348795.
Thanks Ferdinand and Gun for taking care of this massive refactoring! Quite
Review: Needs Fixing
Actually, there are still a bunch of merge conflicts with trunk.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1203629/+merge/228203
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1203629.
Review: Approve
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1341674_codecheck/+merge/227936
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1341674_codecheck.
___
Mailing list: https://launchpad.n
Review: Approve
Refactored a bit, fixed some bugs and merged. I only fixed the bugs that the
tests found - this change touched quite a bit of logic, so it could be that
there are more.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1341081/+merge/227107
Your team Widelands Developer
Review: Approve
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1345663/+merge/228284
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1345663.
___
Mailing list: https://launchpad.net/~widelands-dev
Po
Review: Approve
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1344350/+merge/228379
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1344350.
___
Mailing list: https://launchpad.net/~widelands-dev
Po
I started looking into this now - the testui crashes on game close. and there
are a lot of fixmes in the code that I do not understand and that could use a
comment. I converted them into NOCOM. I also merged trunk.
I will be on vacation starting tuesday in a week for ~3 weeks. Just fyi :)
--
h
Review: Needs Fixing
I added a bunch of code review comments. And this change is awesome!
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1348795/+merge/228430
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1348795.
___
Review: Approve
--
https://code.launchpad.net/~widelands-dev/widelands/help_stringfixes/+merge/228456
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/help_stringfixes.
___
Mailing list: https://launchpad.net/~widela
Review: Approve
This broke the Lua testsuite though :(. I fixed it after merging, would be nice
if you could run ./regression_test.py -b before submitting.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1347648/+merge/228469
Your team Widelands Developers is subscribed to branch
lp
Review: Approve
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1343297/+merge/228552
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1343297.
___
Mailing list: https://launchpad.net/~widelands-dev
Po
Actually m_result_buffer seems unused and can probably be removed. And I see no
reason why m_statistics_buffer is of this particular size - maybe for saving?
However that is no good reason and it can be converted to std::string and
should be.
--
https://code.launchpad.net/~widelands-dev/widelan
Argl... that is why friend and protected are bad. Private members should only
be accessed in the corresponding .cc file. I hope I analyzed this correctly
then.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1348795/+merge/228430
Your team Widelands Developers is subscribed to branch
On 29.07.2014, at 09:41, GunChleoc wrote:
> This is the first time anybody has mentioned that particular Python script to
> me. Is this documented somewhere in the Wiki?
No it isn’t. I do not think it makes sense to document - we had a manual_test
directory before that was very well documente
Review: Needs Fixing
I like the refactorings, but there is at least one bug and a couple of design
suggestions.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1348795/+merge/228430
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1348795.
_
401 - 500 of 1693 matches
Mail list logo
