<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="http://git.reviewboard.kde.org/r/108995/">http://git.reviewboard.kde.org/r/108995/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 28th, 2013, 6:33 p.m. UTC, <b>Harsh Gupta</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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="http://git.reviewboard.kde.org/r/108995/diff/2/?file=119751#file119751line793" style="color: black; font-weight: bold; text-decoration: underline;">src/EngineController.cpp</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; ">EngineController::isEqSupported() const</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">790</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span><span class="p">(</span> <span class="n">mParam</span><span class="p">.</span><span class="n">name</span><span class="p">().</span><span class="n">contains</span><span class="p">(</span> <span class="n">QString</span><span class="p">(</span> <span class="s">"pre-amp"</span> <span class="p">)</span> <span class="p">)</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">793</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span><span class="p">(</span> <span class="o"><span class="hl">!</span></span><span class="n">mParam</span><span class="p">.</span><span class="n">name</span><span class="p">().</span><span class="n">contains</span><span class="p">(</span> <span class="n">QString</span><span class="p">(</span> <span class="s">"pre-amp"</span> <span class="p">)</span> <span class="p">)</span> <span class="p">)</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;">How am I suppose to stage changes in a line ( line 791 ) in which a variable is first renamed and then get deleted ? Should I commit all the changes again in order to make two different patches ?</pre>
</blockquote>
<p>On March 28th, 2013, 6:37 p.m. UTC, <b>Harsh Gupta</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Oops... sorry for poor tagging !!! (first time :P)
Line no. is actually 791.</pre>
</blockquote>
<p>On March 28th, 2013, 6:51 p.m. UTC, <b>Matěj Laitl</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">`git add -p` allows you to edit the hunks while staging, so you can use it to "generate" changes that "annihilated". From "oldVarName" -> "" you can create "oldVarname" -> "newVarName" -> "". (where oldVarName would be in tree, newVarName would be in index (staged to commit) and "" would be in working tree)
But perhaps this is too complicated for an user new to git. You may instead do:
1. start a new branch starting just a commit before your changes: git branch newbranch oldbranch^ (or master^ if you've worked directly in master)
2. perform the exact same rename using your IDE (should be quick to do), commit
3. check out the working tree of oldbranch (master?) without switching to it: git checkout oldbranch -- .
4. `git diff` should show the "real" changes w/thout the renames; commit
5. now you have 2 commits, yay! Ensure that both commits build.</pre>
</blockquote>
<p>On March 31st, 2013, 11:51 p.m. UTC, <b>Harsh Gupta</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Should I submit both the patches in this review request one by one ?
PS: Please ignore my latest patch. I was expecting two different patches (one as a parent diff) but review board merged the two patches.</pre>
</blockquote>
<p>On April 2nd, 2013, 2:35 p.m. UTC, <b>Matěj Laitl</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">> Should I submit both the patches in this review request one by one?
Please submit the renaming patch as new review request here. Indicate in this review request that it depends on the renaming one. (and optionally update it so that it applied on top of the renaming patch - but this seems already done)
> I was just wondering what should I write in the Changelog file?
BUGFIXES:
* Pre-aplifier in equalizer is now only enabled if it is actually supported; patch by Harsh Gupta. (BR 301311)</pre>
</blockquote>
<p>On April 12th, 2013, 11:15 p.m. UTC, <b>Harsh Gupta</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I have created a new review request for the renaming patch. Should I wait for that patch to get shipped ?
PS : Sorry for the late response. University projects kept me occupied :(</pre>
</blockquote>
</blockquote>
<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;">> I have created a new review request for the renaming patch. Should I wait for that patch to get shipped?
It already is. :-)
> PS: Sorry for the late response. University projects kept me occupied :(
No problem, glad to know you weren't hit by a bus. Actual review below.</pre>
<br />
<p>- Matěj</p>
<br />
<p>On April 13th, 2013, 4:14 p.m. UTC, Harsh Gupta wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://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 Amarok.</div>
<div>By Harsh Gupta.</div>
<p style="color: grey;"><i>Updated April 13, 2013, 4:14 p.m.</i></p>
<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;">1. Disabled Pre-amplifier in equalizer if it is not supported by phonon.
2. Fixed top and bottom labels of first slider. Earlier band name label was at the top and slider value label was at the bottom for first slider.
3. Removed an extra semicolon EqualizerDialog.h .
Note : I have made an assumption that if at all preamp is present then it will the first element of Effect Parameter list.</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;">All unit test cases passed.
Note : I have tested it with gstreamer only. Xine phonon keep crashing on my PC.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://bugs.kde.org/show_bug.cgi?id=301311">301311</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>ChangeLog <span style="color: grey">(a278be3)</span></li>
<li>src/EngineController.h <span style="color: grey">(5de4beb)</span></li>
<li>src/EngineController.cpp <span style="color: grey">(28fb256)</span></li>
<li>src/dialogs/EqualizerDialog.cpp <span style="color: grey">(f42a033)</span></li>
<li>src/dialogs/EqualizerDialog.ui <span style="color: grey">(43b0187)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/108995/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments </h1>
<ul>
<li><a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/02/17/equalizer.png">Equalizer snapshot</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>