[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-games-devel
Subject: Re: [Kde-games-devel] Review Request 114974: Add example player to konquest when built in non-releas
From: "Alexander Schuch" <aschuch247 () gmail ! com>
Date: 2014-01-19 1:34:07
Message-ID: 20140119013407.14971.33395 () probe ! kde ! org
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/114974/#review47662
-----------------------------------------------------------
Right now CMAKE_BUILD_TYPE is used to add the example AI CPP files for compilation but the \
symbol NDEBUG is used to actually use the code. Ideally the symbol NDEBUG (actually its non \
presence) should be used as well do exclude the files from compilation. But again, I do not \
know how CMake internally works and whether the symbol is known at that stage already. So if \
you know how to do this, go for it, if not, never mind. :)
dialogs/newgamedlg.cc
<https://git.reviewboard.kde.org/r/114974/#comment33826>
When building a release version, the example AI CPP files aren't compiled and not \
referenced anywhere. So having the header file still enabled here will just make the compiler \
read them - nothing more?
I am not that much into conditional compilation, so no idea if the header files should be \
"excluded" conditionally as well. I leave the decision to you.
- Alexander Schuch
On Jan. 12, 2014, 8:51 p.m., Nemanja Hirsl wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/114974/
> -----------------------------------------------------------
>
> (Updated Jan. 12, 2014, 8:51 p.m.)
>
>
> Review request for KDE Games and Alexander Schuch.
>
>
> Repository: konquest
>
>
> Description
> -------
>
> Just realized that we didn't include example player in (all) debug build types.
> http://techbase.kde.org/Development/Tutorials/Debugging/Debugging_symbols#And_make.3F
>
> Alternative is to set it just for some build types, like:
> - for debug: set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -DKONQUEST_DEBUG")
> - for debugfull set(CMAKE_CXX_FLAGS_DEBUG_FULL "${CMAKE_CXX_FLAGS_DEBUG_FULL} \
> -DKONQUEST_DEBUGFULL") but I'm not sure that we want to do that.
>
>
> Diffs
> -----
>
> CMakeLists.txt a20ce0b
> dialogs/newgamedlg.cc 0485cdd
>
> Diff: https://git.reviewboard.kde.org/r/114974/diff/
>
>
> Testing
> -------
>
> Checked with release, debug and debugfull build types. Example player is not shown in \
> release.
>
> Thanks,
>
> Nemanja Hirsl
>
>
[Attachment #5 (text/html)]
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/114974/">https://git.reviewboard.kde.org/r/114974/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; \
white-space: -o-pre-wrap; word-wrap: break-word;">Right now CMAKE_BUILD_TYPE is used to add the \
example AI CPP files for compilation but the symbol NDEBUG is used to actually use the code. \
Ideally the symbol NDEBUG (actually its non presence) should be used as well do exclude the \
files from compilation. But again, I do not know how CMake internally works and whether the \
symbol is known at that stage already. So if you know how to do this, go for it, if not, never \
mind. :)</pre> <br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; \
border-collapse: collapse; margin: 2px padding: 2px;"> <thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; \
padding: 4px 8px; text-align: left;"> <a \
href="https://git.reviewboard.kde.org/r/114974/diff/2/?file=233883#file233883line30" \
style="color: black; font-weight: bold; text-decoration: underline;">dialogs/newgamedlg.cc</a> \
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; \
">NewGameDlg::NewGameDlg( QWidget *parent, Game *game)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font \
size="2">30</font></th> <td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; \
line-height: 140%; margin: 0; "><span class="cp">#include \
"../players/ai/example/example_gui.h"</span></pre></td> <th bgcolor="#f0f0f0" \
style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font \
size="2">30</font></th> <td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; \
line-height: 140%; margin: 0; "><span class="cp">#include \
"../players/ai/example/example_gui.h"</span></pre></td> </tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">When building a release version, \
the example AI CPP files aren't compiled and not referenced anywhere. So having the header \
file still enabled here will just make the compiler read them - nothing more?
I am not that much into conditional compilation, so no idea if the header files should be \
"excluded" conditionally as well. I leave the decision to you.</pre> </div>
<br />
<p>- Alexander Schuch</p>
<br />
<p>On January 12th, 2014, 8:51 p.m. UTC, Nemanja Hirsl wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: \
url('https://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); \
background-position: left top; background-repeat: repeat-x; border: 1px black solid;"> <tr>
<td>
<div>Review request for KDE Games and Alexander Schuch.</div>
<div>By Nemanja Hirsl.</div>
<p style="color: grey;"><i>Updated Jan. 12, 2014, 8:51 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
konquest
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px \
solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Just realized that we \
didn't include example player in (all) debug build types. \
http://techbase.kde.org/Development/Tutorials/Debugging/Debugging_symbols#And_make.3F
Alternative is to set it just for some build types, like:
- for debug: set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -DKONQUEST_DEBUG")
- for debugfull set(CMAKE_CXX_FLAGS_DEBUG_FULL "${CMAKE_CXX_FLAGS_DEBUG_FULL} \
-DKONQUEST_DEBUGFULL") but I'm not sure that we want to do that.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid \
#b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Checked with release, \
debug and debugfull build types. Example player is not shown in release.</pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>CMakeLists.txt <span style="color: grey">(a20ce0b)</span></li>
<li>dialogs/newgamedlg.cc <span style="color: grey">(0485cdd)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/114974/diff/" style="margin-left: 3em;">View \
Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
_______________________________________________
kde-games-devel mailing list
kde-games-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-games-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic