<div><div><div dir="auto">Pino.</div><div dir="auto"><br></div><div dir="auto"> I understand that, also please understand that he's a newcomer and he's still learning the tools and being increasingly difficult to accept a review (that can be fixed in a follow-up review if the error is not blatant) is counter productive and demotivating.</div></div></div><div dir="auto"><br></div><div dir="auto">We have no commits in kde edu for a while, IV found people to help, let the commits pass even if he closed the original review and opened a new one. The code is correct.</div><div dir="auto"><br></div><div dir="auto"><br></div><div dir="auto"><br></div><div><div><br><div class="gmail_quote"><div dir="ltr">Em sex, 12 de out de 2018 às 07:23, Pino Toscano <<a href="mailto:noreply@phabricator.kde.org" target="_blank">noreply@phabricator.kde.org</a>> escreveu:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><table><tbody><tr><td>pino added a comment.
</td><td><a style="text-decoration:none;padding:4px 8px;margin:0 8px 8px;float:right;color:#464c5c;font-weight:bold;border-radius:3px;background-color:#f7f7f9;background-image:linear-gradient(to bottom,#fff,#f1f0f1);display:inline-block;border:1px solid rgba(71,87,120,.2)" href="https://phabricator.kde.org/D16132" target="_blank">View Revision</a></td></tr></tbody></table><br><div><div><blockquote style="border-left:3px solid #8c98b8;color:#6b748c;font-style:italic;margin:4px 0 12px 0;padding:8px 12px;background-color:#f8f9fc">
<div style="font-style:normal;padding-bottom:4px">In <a href="https://phabricator.kde.org/D16132#341566" style="background-color:#e7e7e7;border-color:#e7e7e7;border-radius:3px;padding:0 4px;font-weight:bold;color:black;text-decoration:line-through" target="_blank">D16132#341566</a>, <a href="https://phabricator.kde.org/p/filipesaraiva/" style="border-color:#f1f7ff;color:#19558d;background-color:#f1f7ff;border:1px solid transparent;border-radius:3px;font-weight:bold;padding:0 4px" target="_blank">@filipesaraiva</a> wrote:</div>
<div style="margin:0;padding:0;border:0;color:rgb(107,116,140)"><p>This patch was splitted from <a href="https://phabricator.kde.org/D16120" style="background-color:#e7e7e7;border-color:#e7e7e7;border-radius:3px;padding:0 4px;font-weight:bold;color:black;text-decoration:none" target="_blank">D16120</a> because he was trying to create different reviews for different changes. Were you asking for it, no?</p></div>
</blockquote>

<p>In <a href="https://phabricator.kde.org/D16120" style="background-color:#e7e7e7;border-color:#e7e7e7;border-radius:3px;padding:0 4px;font-weight:bold;color:black;text-decoration:none" target="_blank">D16120: Changed old style casts</a> I asked to split changes which were not "fix old style casts", and <a href="https://phabricator.kde.org/D16140" style="background-color:#e7e7e7;border-color:#e7e7e7;border-radius:3px;padding:0 4px;font-weight:bold;color:black;text-decoration:none" target="_blank">D16140: Removed unreachable lines</a> does exactly that.<br>
This is basic commit hygene:</p>

<ul class="m_292527538535901749m_1325551294453226191remarkup-list">
<li class="m_292527538535901749m_1325551294453226191remarkup-list-item">have unrelated changes in different commits, so they can be reviewed & tested more easily</li>
<li class="m_292527538535901749m_1325551294453226191remarkup-list-item">document properly what are the changes in a commit</li>
</ul>

<p><a href="https://phabricator.kde.org/p/carlos_hdc/" style="border-color:#f1f7ff;color:#19558d;background-color:#f1f7ff;border:1px solid transparent;border-radius:3px;font-weight:bold;padding:0 4px" target="_blank">@carlos_hdc</a> started <a href="https://phabricator.kde.org/D16120" style="background-color:#e7e7e7;border-color:#e7e7e7;border-radius:3px;padding:0 4px;font-weight:bold;color:black;text-decoration:none" target="_blank">D16120: Changed old style casts</a> (or actually <a href="https://phabricator.kde.org/D15690" style="background-color:#e7e7e7;border-color:#e7e7e7;border-radius:3px;padding:0 4px;font-weight:bold;color:black;text-decoration:line-through" target="_blank">D15690: Removed old style casts</a>, which was abandoned instead of just being fixed...) about "fix old style casts": that is good, so do not lump it with unrelated changes, such as whitespaces changes, remove unreacheable lines, and so forth.</p></div></div><br><div><strong>REPOSITORY</strong><div><div>R326 Kalzium</div></div></div><br><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D16132" target="_blank">https://phabricator.kde.org/D16132</a></div></div><br><div><strong>To: </strong>carlos_hdc, tcanabrava, filipesaraiva, pino<br><strong>Cc: </strong>pino, kde-edu, narvaez, apol<br></div></blockquote></div></div>
</div>