<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/127910/">https://git.reviewboard.kde.org/r/127910/</a>
</td>
</tr>
</table>
<br />
<p>
Fix it, then Ship it!
</p>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Looks good aside from one remaining issue!</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/127910/diff/3/?file=464808#file464808line45" style="color: black; font-weight: bold; text-decoration: underline;">modules/ksb/RecursiveFH.pm</a>
<span style="font-weight: normal;">
(Diff revision 3)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">40</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">my</span> <span class="nv">$result</span> <span class="o">=</span> <span class="nb">pop</span> <span class="nv">@</span><span class="p">{</span><span class="nv">$self</span><span class="o">-></span><span class="p">{</span><span class="n">filehandles</span><span class="p">}};</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">45</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">my</span> <span class="nv">$result</span> <span class="o">=</span> <span class="nb">pop</span> <span class="nv">@</span><span class="p">{</span><span class="nv">$self</span><span class="o">-></span><span class="p">{</span><span class="n">filehandles</span><span class="p">}}</span><span class="hl"> </span><span class="o"><span class="hl">&&</span></span><span class="hl"> </span><span class="nb"><span class="hl">pop</span></span><span class="hl"> </span><span class="nv"><span class="hl">@</span></span><span class="p"><span class="hl">{</span></span><span class="nv"><span class="hl">$self</span></span><span class="o"><span class="hl">-></span></span><span class="p"><span class="hl">{</span></span><span class="n"><span class="hl">filenames</span></span><span class="p"><span class="hl">}}</span>;</span></pre></td>
</tr>
</tbody>
</table>
<div style="margin-left: 2em;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">`$result` here is the old filehandle (so it can be closed) so this boolean won't work.
If you wanted to be fancy you could use comma operator perhaps, but it's probably best to just put the filename stack update on a separate line.
Alternately we could close the fh here and free up the calling code from doing it, and then return nothing... might be clearer that way.</pre>
</div>
</div>
<br />
<p>- Michael Pyne</p>
<br />
<p>On May 15th, 2016, 12:59 p.m. UTC, David Faure wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for Build System and Michael Pyne.</div>
<div>By David Faure.</div>
<p style="color: grey;"><i>Updated May 15, 2016, 12:59 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kdesrc-build
</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;">Before:
"Don't use module libaccounts-qt on line 20 of /path/kdesrc-buildrc, use options libaccounts-qt"
but line 20 is unrelated, some global option.
After:
"Don't use module libaccounts-qt on line 20 of /path/extragear/utils/kdesrc-build/kf5-workspace-build-include, use options libaccounts-qt"</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>modules/ksb/Application.pm <span style="color: grey">(5dbd224c7ba06242b53cd77cfa0764c28da76579)</span></li>
<li>modules/ksb/RecursiveFH.pm <span style="color: grey">(6892320bb5e8f8c1e4979ef137bb975775940908)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/127910/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>