The proposal to merge lp:~widelands-dev/widelands/bug-1348795 into lp:widelands
has been updated.
Status: Needs review => Merged
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1348795/+merge/228430
--
https://code.launchpad.net/~widelands-dev/widelands/bug-13
I have put some questions in NOCOM comments that I need help with
--
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.
___
Maili
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.
_
Review: Resubmit
This is ready for review again.
- Changed 2 protected char* buffers to private strings.
- Added "Out Of Fields" etc. tooltip
- Refactored ProductionProgram::ActMine::notify_player and
ProductionSite::worker_failed_to_find_resource
- Fixed codecheck warnings & NOCOMs
--
http
I can just bzr revert if it blows up :)
--
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.
___
Mailing list: https://launchpad
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
Actually, m_result_buffer is the hover tooltip for the buildings ;)
I'll go change and try to make sure that it doesn't break saving.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1348795/+merge/228430
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/wideland
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
OK. this should be mergeable now.
There is one thing I was wondering though: why is m_result_buffer in
productionsite.cc of type char[213] rather than std::string? I don't understand
the relevance of the type for the map_io object, which has a buffer length
check in it.
I'd prefer a dynamic le
Please wait a bit with meging, I've had some ideas for refactoring
--
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.
___
Mail
Review: Resubmit
Yes it is, I have been wanting this from the beginning and now I finally have
the C++ skills (and IDE :P) to pull this off.
Depending on what our translators say, we might still need to do further tweaks
in the future (e.g. on the summing for is/are - languages might handle and
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
LGTM.
I've compiled the code and did a quick check of some new tooltip texts of
buildings
--
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.
___
GunChleoc has proposed merging lp:~widelands-dev/widelands/bug-1348795 into
lp:widelands.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1310774 in widelands: "Problems with translating "%1$s %2$s mine %3$s""
https://bugs.launchpad.net/widelands/+bug/1310774
Bu
14 matches
Mail list logo
