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
> another layer of compatibility code, which I'd rather avoid.
Could this be in one of those cases where polishing the English should be done
at Widelands-to British/American/whatever translations instead of the Widelands
code itself?
--
https://code.launchpad.net/~widelands-dev/widelands/fix
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 more details, see
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
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
The proposal to merge lp:~widelands-dev/widelands/tibor-ai4 into lp:widelands
has been updated.
Status: Needs review => Merged
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/tibor-ai4/+merge/226029
--
https://code.launchpad.net/~widelands-dev/widelands/tibor-ai4/
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.
The proposal to merge lp:~widelands-dev/widelands/bug-1330599 into lp:widelands
has been updated.
Status: Needs review => Merged
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1330599/+merge/226607
--
https://code.launchpad.net/~widelands-dev/widelands/bug-13
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
The proposal to merge lp:~widelands-dev/widelands/editor_tweaks into
lp:widelands has been updated.
Status: Needs review => Merged
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/editor_tweaks/+merge/226606
--
https://code.launchpad.net/~widelands-dev/widelands/ed
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: 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
--
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
The proposal to merge lp:~widelands-dev/widelands/bug-1341112 into lp:widelands
has been updated.
Status: Needs review => Merged
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1341112/+merge/226586
--
https://code.launchpad.net/~widelands-dev/widelands/bug-13
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
GunChleoc has proposed merging lp:~widelands-dev/widelands/bug-1330599 into
lp:widelands.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1330599 in widelands: "Forbid SDL integer types in codecheck and remove
its use"
https://bugs.launchpad.net/widelands/+bug/13
Jens Beyer has proposed merging lp:~qcumber-some/widelands/cmake-unclutter into
lp:widelands.
Requested reviews:
Widelands Developers (widelands-dev)
For more details, see:
https://code.launchpad.net/~qcumber-some/widelands/cmake-unclutter/+merge/226602
Leaving this here for your judgement.
17 matches
Mail list logo
