Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1784200-single-line-escaping into lp:widelands

2018-08-21 Thread GunChleoc
Review: Approve Code LGTM, not tested yet. I have added some string fixes. Links should not be translatable, and all links need to point to our own site, because we're planning to mover to GitHub after Build 20 comes out, and the Launchpad links will become invalid. -- https://code.launchpad.n

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/mines_sleep_time into lp:~widelands-dev/widelands/mines-worldsavior

2018-08-21 Thread hessenfarmer
Review: Approve thanks for doing this. Code LGTM ;-) -- https://code.launchpad.net/~widelands-dev/widelands/mines_sleep_time/+merge/353450 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/mines-worldsavior. ___ Mai

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/mines_sleep_time into lp:~widelands-dev/widelands/mines-worldsavior

2018-08-21 Thread hessenfarmer
@bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/mines_sleep_time/+merge/353450 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/mines-worldsavior. ___ Mailing list: https://launchpad.net/~widelan

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/fix_osx_nightlies into lp:widelands

2018-08-21 Thread GunChleoc
This has broken all the Linux builds on Travis: -- Found ICU: /usr/lib/x86_64-linux-gnu/libicuuc.so (found version "52.1.0") CMake Error at CMakeLists.txt:84 (if): if given arguments: "APPLE" "AND" "GREATER" "11" Unknown arguments specified -- Configuring incomplete, errors occurred! --

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/mines_sleep_time into lp:~widelands-dev/widelands/mines-worldsavior

2018-08-21 Thread GunChleoc
Bunnybot can only handle mergers to trunk, so we need to wait for Travis and then merge manually. -- https://code.launchpad.net/~widelands-dev/widelands/mines_sleep_time/+merge/353450 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/mines-worldsavior. _

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1784200-single-line-escaping into lp:widelands

2018-08-21 Thread Notabilis
Thanks for the review. Interesting to know that we will move to GitHub. Since we are now only linking to instructions about how to report a bug, maybe change "Please report bugs at:" to "For instructions about how to report bugs, see:" ? -- https://code.launchpad.net/~widelands-dev/widelands/bu

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe into lp:widelands

2018-08-21 Thread Notabilis
Review: Approve Code is looking okay and the editor no longer crashes. Two observations: - When loading the old/broken maps, we still get the "Tribe not known" message. Maybe only display a "note:" message and select a default tribe in that case? Having the wrong tribe is in most cases probably

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1786613-10s-return-skipped into lp:widelands

2018-08-21 Thread hessenfarmer
Review: Disapprove This solution doesn't work for the atlantean Crystal mine and the empire marblemine in their new design as defined in "mines-worldsavior" branch. In principle there may be a misunderstanding about the "work" program. This is the master program of each productionsite. Meaning

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/mines_sleep_time into lp:~widelands-dev/widelands/mines-worldsavior

2018-08-21 Thread hessenfarmer
Ah good to know. I was suspecting it but didn't know. So for the future I would prefer not to have any merge requests into this branch. @Toni Förster: As I really appreciate your work probably it would be easier for the future to pull the branch do the agreed changes right in there and push a ne

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/mines_sleep_time into lp:~widelands-dev/widelands/mines-worldsavior

2018-08-21 Thread Toni Förster
I if you want I can merge this into world-saviors branch myself. -- https://code.launchpad.net/~widelands-dev/widelands/mines_sleep_time/+merge/353450 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/mines-worldsavior. ___

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/fix_osx_nightlies into lp:widelands

2018-08-21 Thread GunChleoc
I have now pushed a fix for Linux. Does it still work for the OSX nightlies? -- https://code.launchpad.net/~widelands-dev/widelands/fix_osx_nightlies/+merge/353456 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/fix_osx_nightlies into lp:wid

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/mines_sleep_time into lp:~widelands-dev/widelands/mines-worldsavior

2018-08-21 Thread hessenfarmer
I would appreciate this. However I don't know what are the implications with travis that GunChleoc mentioned. So I am not sure if we have to wait for travis to do so. -- https://code.launchpad.net/~widelands-dev/widelands/mines_sleep_time/+merge/353450 Your team Widelands Developers is subscrib

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1784200-single-line-escaping into lp:widelands

2018-08-21 Thread GunChleoc
How about "For reporting bugs, visit:"? Too keep it short. I have done some testing now and it's working, so this branch can go in once we have agreed on the string. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1784200-single-line-escaping/+merge/353446 Your team Widelands Develope

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/fix_osx_nightlies into lp:widelands

2018-08-21 Thread Toni Förster
Review: Approve > I have now pushed a fix for Linux. Does it still work for the OSX nightlies? It does for me. -- https://code.launchpad.net/~widelands-dev/widelands/fix_osx_nightlies/+merge/353456 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/fix_osx_nightl

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1786613-10s-return-skipped into lp:widelands

2018-08-21 Thread Toni Förster
New revision. We check whether the program is "work" or end with "work". This would work with the marble mines as well. Now if a program shall not calculate stats, only append _work at the end. Here is the aforementioned marble-mine: work = { -- TRANSLATORS: Completed/Skipped/Did

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1786613-10s-return-skipped into lp:widelands

2018-08-21 Thread hessenfarmer
Ok I see your point about the stack. However your solution would need a lot of changes I the programs and additionally we would need to document the rule to implicitly avoid stats calculation by adding_work to the end of the program. So I was thinking why not introduce a way to explicitly state

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/fix_osx_nightlies into lp:widelands

2018-08-21 Thread bunnybot
Continuous integration builds have changed state: Travis build 3825. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/418587526. Appveyor build 3624. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_fix_osx_nig

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1786613-10s-return-skipped into lp:widelands

2018-08-21 Thread Toni Förster
The proposal to merge lp:~widelands-dev/widelands/bug-1786613-10s-return-skipped into lp:widelands has been updated. Status: Needs review => Superseded For more details, see: https://code.launchpad.net/~widelands-dev/widelands/bug-1786613-10s-return-skipped/+merge/353449 -- Your team Widel

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1786613-10s-return-skipped into lp:widelands

2018-08-21 Thread Toni Förster
Toni Förster has proposed merging lp:~widelands-dev/widelands/bug-1786613-10s-return-skipped into lp:widelands with lp:~widelands-dev/widelands/mines-worldsavior as a prerequisite. Commit message: introduced the return value no_stats for work programs. Requested reviews: hessenfarmer (stephan

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1786613-10s-return-skipped into lp:widelands

2018-08-21 Thread Toni Förster
Ok, I'm pretty sure that this is the final solution :-D Unconditional "return=skipped" need to be replaced with "return=no_stats". That's it no stats are calculated and it also handles the conditional skips. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1786613-10s-return-skipped/+m

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/multiplayer-ui into lp:widelands

2018-08-21 Thread Toni Förster
Toni Förster has proposed merging lp:~widelands-dev/widelands/multiplayer-ui into lp:widelands. Commit message: multiplayer-ui fine tuning Requested reviews: Widelands Developers (widelands-dev) For more details, see: https://code.launchpad.net/~widelands-dev/widelands/multiplayer-ui/+merge/3

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1784122-singleplayer-viewport into lp:widelands

2018-08-21 Thread Notabilis
Review: Approve Code is looking good and working as intended. That definitely is one nasty bug... -- https://code.launchpad.net/~widelands-dev/widelands/bug-1784122-singleplayer-viewport/+merge/353391 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1784122

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1786613-10s-return-skipped into lp:widelands

2018-08-21 Thread hessenfarmer
Yes. Looks reasonable now. Needs to be tested though. From which branch did you split this one? If not from Mines_worldsavior we would probably run into a merge conflict, as we did some changes in productionsite.cc in this branch as well. So I will not approve this now although I'm happy with th

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1786613-10s-return-skipped into lp:widelands

2018-08-21 Thread Toni Förster
I branched this one from trunk. That's why this branch depends on mines-worldsavior. As soon as that one gets merged into trunk, I'm gone rebase this one here, to avoid merge conflicts. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1786613-10s-return-skipped/+merge/353514 Your team

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/mines_sleep_time into lp:~widelands-dev/widelands/mines-worldsavior

2018-08-21 Thread noreply
The proposal to merge lp:~widelands-dev/widelands/mines_sleep_time into lp:~widelands-dev/widelands/mines-worldsavior has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/mines_sleep_time/+merge/353450 -- Your team Widel

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1787105_tada_edited_sounds into lp:widelands

2018-08-21 Thread bunnybot
Continuous integration builds have changed state: Travis build 3826. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/418808436. Appveyor build 3625. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1787105

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/multiplayer-ui into lp:widelands

2018-08-21 Thread bunnybot
Continuous integration builds have changed state: Travis build 3827. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/418836717. Appveyor build 3626. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_multiplaye

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1786613-10s-return-skipped into lp:widelands

2018-08-21 Thread Toni Förster
I just merged the mines-worldsavior into this one and replaced the return=skipped with return=no_stats. So as soon as worldsavior's branch gets merged into trunk this one here is ready to go. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1786613-10s-return-skipped/+merge/353514 Your

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/mines-worldsavior into lp:widelands

2018-08-21 Thread bunnybot
Continuous integration builds have changed state: Travis build 3828. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/418892074. Appveyor build 3627. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_mines_worl

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/fix_osx_nightlies into lp:widelands

2018-08-21 Thread SirVer
Review: Approve @stonerl: There is no reason to apologize - I am super excited that you picked up, undusted and improved the Mac OS X build experience and untested things break with change. I am sorry to partially undo your modernizations efforts, but I do not have time to do more major changes

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/fix_osx_nightlies into lp:widelands

2018-08-21 Thread noreply
The proposal to merge lp:~widelands-dev/widelands/fix_osx_nightlies into lp:widelands has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/fix_osx_nightlies/+merge/353456 -- Your team Widelands Developers is subscribed t