[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-games-devel
Subject: Re: [Kde-games-devel] Review Request 125992: fix homepage link: Insert empty otherText in KAboutData
From: Jakob Gruber <jakob.gruber () gmail ! com>
Date: 2015-11-11 8:25:15
Message-ID: CANj_-imWk1_rpy4mU1+z2bp1L2B7p3-4qkOkTtQ7mFcHmh+CEA () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
Sorry for the late reply, only just noticed this. I also prefer Frederik's
version, but either way looks good to me.
Thanks for the patch!
Jakob
On Wed, Nov 11, 2015 at 9:19 AM, Frederik Schwarzer <schwarzer@kde.org>
wrote:
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125992/
>
> On November 8th, 2015, 2:13 a.m. UTC, *Frederik Schwarzer* wrote:
>
> src/main.cpp
> <https://git.reviewboard.kde.org/r/125992/diff/1/?file=415723#file415723line50> \
> (Diff revision 1)
>
> int main(int argc, char *argv[])
>
> 49
>
> "https://projects.kde.org/projects/kde/kdegames/picmi");
>
> 50
>
> "https://projects.kde.org/projects/kde/kdegames/picmi");
>
> I know, not directly related to your change, but maybe Jakob can answer, why the \
> link goes projects.kde.org, while there is something more colourful like \
> https://games.kde.org/picmi ? :)
> Other than that, your fix is valid. ... ... although :D
>
> It might be more readable to give the constructor only everything until the \
> copyright info and then use .setHomepage() to set the URL.
> Here the (simplified) code from KShisen:
>
> KAboutData aboutData(QStringLiteral("kshisen"), i18n("Shisen-Sho"),
> QStringLiteral(KSHISEN_VERSION),
> i18n(description), KAboutLicense::GPL,
> i18n("(c)"));
>
> aboutData.setHomepage(QStringLiteral("http://games.kde.org/kshisen"));
>
> Since Jakob does notr seem to read this, can you please try to reach him directly?
>
> As for the link fix change, please commit it. Either in your version or mine, \
> whatever feels better to you.
>
> - Frederik
>
> On November 8th, 2015, 1:33 a.m. UTC, Martin Walch wrote:
> Review request for KDE Games.
> By Martin Walch.
>
> *Updated Nov. 8, 2015, 1:33 a.m.*
> *Repository: * picmi
> Description
>
> The hyperlink in >Help >About Picmi does not work, because the hyperlink
> is put in the place of the additional free form text for that dialog.
>
> This was probably an accident in commit 7ab84ae5b3a... when porting to
> KF5. Fix it by inserting a QString().
>
> Testing
>
> Compiling, Running. Clicking on the hyperlink.
>
> Diffs
>
> - src/main.cpp (ec3c5f259bfa4b2e76b3f14231ce79eebd1b7fae)
>
> View Diff <https://git.reviewboard.kde.org/r/125992/diff/>
>
> _______________________________________________
> kde-games-devel mailing list
> kde-games-devel@kde.org
> https://mail.kde.org/mailman/listinfo/kde-games-devel
>
>
[Attachment #5 (text/html)]
<div dir="ltr"><div>Sorry for the late reply, only just noticed this. I also prefer \
Frederik's version, but either way looks good to me. <br><br>Thanks for the \
patch!<br></div>Jakob<br></div><div class="gmail_extra"><br><div \
class="gmail_quote">On Wed, Nov 11, 2015 at 9:19 AM, Frederik Schwarzer <span \
dir="ltr"><<a href="mailto:schwarzer@kde.org" \
target="_blank">schwarzer@kde.org</a>></span> wrote:<br><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex">
<div>
<div style="font-family:Verdana,Arial,Helvetica,Sans-Serif"><span class="">
<table style="border:1px #c9c399 solid;border-radius:6px" bgcolor="#f9f3c9" \
cellpadding="12" width="100%"> <tbody><tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/125992/" \
target="_blank">https://git.reviewboard.kde.org/r/125992/</a> </td>
</tr>
</tbody></table>
<br>
</span><span class=""><blockquote style="margin-left:1em;border-left:2px solid \
#d0d0d0;padding-left:10px"> <p style="margin-top:0">On November 8th, 2015, 2:13 a.m. \
UTC, <b>Frederik Schwarzer</b> wrote:</p> <blockquote \
style="margin-left:1em;border-left:2px solid #d0d0d0;padding-left:10px">
<table bgcolor="white" border="0" width="100%">
<thead>
<tr>
<th colspan="4" style="border-bottom:1px solid #c0c0c0;font-size:9pt;padding:4px \
8px;text-align:left" bgcolor="#F0F0F0"> <a \
href="https://git.reviewboard.kde.org/r/125992/diff/1/?file=415723#file415723line50" \
style="color:black;font-weight:bold;text-decoration:underline" \
target="_blank">src/main.cpp</a> <span style="font-weight:normal">
(Diff revision 1)
</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">int main(int \
argc, char *argv[])</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th style="border-right:1px solid #c0c0c0" bgcolor="#f0f0f0" align="right"><font \
size="2">49</font></th> <td bgcolor="#ffffff" width="50%"><pre \
style="font-size:8pt;line-height:140%;margin:0"> <span>"<a \
href="https://projects.kde.org/projects/kde/kdegames/picmi" \
target="_blank">https://projects.kde.org/projects/kde/kdegames/picmi</a>"</span><span>);</span></pre></td>
<th style="border-left:1px solid #c0c0c0;border-right:1px solid #c0c0c0" \
bgcolor="#f0f0f0" align="right"><font size="2">50</font></th> <td bgcolor="#ffffff" \
width="50%"><pre style="font-size:8pt;line-height:140%;margin:0"> \
<span>"<a href="https://projects.kde.org/projects/kde/kdegames/picmi" \
target="_blank">https://projects.kde.org/projects/kde/kdegames/picmi</a>"</span><span>);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space:pre-wrap;white-space:-moz-pre-wrap;white-space:-pre-wrap;white-space:-o-pre-wrap;word-wrap:break-word"><p \
style="padding:0;margin:0;line-height:inherit;white-space:inherit">I know, not \
directly related to your change, but maybe Jakob can answer, why the link goes <a \
href="http://projects.kde.org" target="_blank">projects.kde.org</a>, while there is \
something more colourful like <a href="https://games.kde.org/picmi" \
target="_blank">https://games.kde.org/picmi</a> ? :)</p> <p \
style="padding:0;margin:0;line-height:inherit;white-space:inherit">Other than that, \
your fix is valid. ... ... although :D</p> <p \
style="padding:0;margin:0;line-height:inherit;white-space:inherit">It might be more \
readable to give the constructor only everything until the copyright info and then \
use .setHomepage() to set the URL.</p> <p \
style="padding:0;margin:0;line-height:inherit;white-space:inherit">Here the \
(simplified) code from KShisen:</p> <p \
style="padding:0;margin:0;line-height:inherit;white-space:inherit"></p><div \
style="background:#f8f8f8"><pre style="line-height:125%">KAboutData <span \
style="color:#0000ff">aboutData</span>(QStringLiteral(<span \
style="color:#ba2121">"kshisen"</span>), i18n(<span \
style="color:#ba2121">"Shisen-Sho"</span>), \
QStringLiteral(KSHISEN_VERSION), i18n(description), KAboutLicense<span \
style="color:#666666">::</span>GPL, i18n(<span \
style="color:#ba2121">"(c)"</span>));
aboutData.setHomepage(QStringLiteral(<span style="color:#ba2121">"<a \
href="http://games.kde.org/kshisen" \
target="_blank">http://games.kde.org/kshisen</a>"</span>)); </pre></div>
<p></p></pre>
</blockquote>
</blockquote>
</span><pre style="margin-left:1em;white-space:pre-wrap;white-space:-moz-pre-wrap;white-space:-pre-wrap;white-space:-o-pre-wrap;word-wrap:break-word"><p \
style="padding:0;margin:0;line-height:inherit;white-space:inherit">Since Jakob does \
notr seem to read this, can you please try to reach him directly?</p> <p \
style="padding:0;margin:0;line-height:inherit;white-space:inherit">As for the link \
fix change, please commit it. Either in your version or mine, whatever feels better \
to you.</p></pre><span class="HOEnZb"><font color="#888888"> <br>
<p>- Frederik</p></font></span><span class="">
<br>
<p>On November 8th, 2015, 1:33 a.m. UTC, Martin Walch wrote:</p>
<table style="border:1px #888a85 solid;border-radius:6px" bgcolor="#fefadf" \
cellpadding="12" cellspacing="0" width="100%"> <tbody><tr>
<td>
<div>Review request for KDE Games.</div>
<div>By Martin Walch.</div>
<p style="color:grey"><i>Updated Nov. 8, 2015, 1:33 a.m.</i></p>
<div style="margin-top:1.5em">
<b style="color:#575012;font-size:10pt">Repository: </b>
picmi
</div>
<h1 style="color:#575012;font-size:10pt;margin-top:1.5em">Description </h1>
<table style="border:1px solid #b8b5a0" bgcolor="#ffffff" cellpadding="10" \
cellspacing="0" width="100%"> <tbody><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"><p \
style="padding:0;margin:0;line-height:inherit;white-space:inherit">The hyperlink in \
>Help >About Picmi does not work, because the hyperlink is put in the place of \
the additional free form text for that dialog.</p> <p \
style="padding:0;margin:0;line-height:inherit;white-space:inherit">This was probably \
an accident in commit 7ab84ae5b3a... when porting to KF5. Fix it by inserting a \
QString().</p></pre> </td>
</tr>
</tbody></table>
<h1 style="color:#575012;font-size:10pt;margin-top:1.5em">Testing </h1>
<table style="border:1px solid #b8b5a0" bgcolor="#ffffff" cellpadding="10" \
cellspacing="0" width="100%"> <tbody><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"><p \
style="padding:0;margin:0;line-height:inherit;white-space:inherit">Compiling, \
Running. Clicking on the hyperlink.</p></pre> </td>
</tr>
</tbody></table>
<h1 style="color:#575012;font-size:10pt;margin-top:1.5em">Diffs </h1>
<ul style="margin-left:3em;padding-left:0">
<li>src/main.cpp <span \
style="color:grey">(ec3c5f259bfa4b2e76b3f14231ce79eebd1b7fae)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/125992/diff/" style="margin-left:3em" \
target="_blank">View Diff</a></p>
</td>
</tr>
</tbody></table>
</span></div>
</div>
<br>_______________________________________________<br>
kde-games-devel mailing list<br>
<a href="mailto:kde-games-devel@kde.org">kde-games-devel@kde.org</a><br>
<a href="https://mail.kde.org/mailman/listinfo/kde-games-devel" rel="noreferrer" \
target="_blank">https://mail.kde.org/mailman/listinfo/kde-games-devel</a><br> \
<br></blockquote></div><br></div>
[Attachment #6 (text/plain)]
_______________________________________________
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