Finally we got this!!!
Thanks GunChleoc :-)
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1358880-ship-statistics/+merge/343293
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/ships_optr.
___
Mailing
The proposal to merge lp:~widelands-dev/widelands/bug-1358880-ship-statistics
into lp:widelands has been updated.
Status: Needs review => Merged
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1358880-ship-statistics/+merge/343293
--
Your team Widela
Transient failure on Travis
@bunnybot merge force
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1358880-ship-statistics/+merge/343293
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/ships_optr.
___
Refusing to merge, since Travis is not green. Use @bunnybot merge force for
merging anyways.
Travis build 3386. State: failed. Details:
https://travis-ci.org/widelands/widelands/builds/367231513.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1358880-ship-statistics/+merge/343293
Thanks for the review and testing again!
I am still tweaking my IDE's layout function as best as I can. bunnybot will
take care of the misaligned assert :)
@bunnybot merge
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1358880-ship-statistics/+merge/343293
Your team Widelands
Review: Approve
Thanks for the fixes/changes! Testing and reviewing the commits went fine.
One really minor nit: Some indentations are now not correctly aligned (some
assert() somewhere). ;)
Unfortunately I don't know any steps to reproduce the memory leak. It didn't
seem to turn up every time
Review: Resubmit
I have now fixed everything except maybe for this one:
- Memory leak with create_shipinfo
I thought I had caught that one already, but I have worked on the function just
in case - it now returns a unique_ptr. That function had also produced a
compiler warning which I fixed
I guess I would have called it "Ship List" or something like that. But
consistency is a good argument, lets leave it as it is.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1358880-ship-statistics/+merge/343293
Your team Widelands Developers is subscribed to branch
Thanks for the review! Everything that I haven't added a comment to, I will
implement exactly as you suggested.
As to the name - I guess that's for consistency, just like we have the
"Building Statistics", which is also a mix of info and unit access. I am open
to suggestions though :)
Diff
Review: Needs Fixing
Great work, I really like the new list!
Most of the code looks good, three small bugs and some other comments, though.
The bugs:
- Hotkey 'e' even works on non-seafaring maps
- Missing "ship name" column in the list when there currently are no ships
- Memory leak with
/_widelands_dev_widelands_bug_1358880_ship_statistics-3192.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1358880-ship-statistics/+merge/343293
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/bug-1358880-ship-statistics into lp:widelands
to go in first.
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/bug-1358880-ship-statistics into lp:widelands.
=== added file 'data/images/wui/editor/fsel_editor_set_port_space.png'
Binary files data/images/wui/editor/fsel_editor_set_port_s
12 matches
Mail list logo
