<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/100806/">http://git.reviewboard.kde.org/r/100806/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On May 25th, 2011, 7:28 p.m., <b>Ralf Engels</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;">The description of the configuration option seems to be wrong.
It says "use composer field" where in reality it is "using the composer field as artist" which only makes sense for classical tracks.
Am I missing something here?</pre>
</blockquote>
<p>On May 25th, 2011, 7:56 p.m., <b>Nicholas Wilson</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;">Nope, not missing anything. That's right. The only way to use the composer field in scrobbling is to send it as the artist. That's why it was suggested that there be an option for it. Note that this is how last.fm itself stores the data: it picks album art and pictures of the composers etc based on composer as artist.</pre>
</blockquote>
<p>On May 25th, 2011, 8:36 p.m., <b>Ralf Engels</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;">Ok.
If you agree then I will submit the patch and update the description of the configuration option.</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Thanks. Maybe I'll get round to submitting the patches for the random albums navigator bugs now that the last few patches have gone through. That's pleasing—once I get the patches for all the bugs bothering me upstream I can go back to using distro packages again.</pre>
<br />
<p>- Nicholas</p>
<br />
<p>On March 6th, 2011, 12:34 a.m., Nicholas Wilson wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Amarok.</div>
<div>By Nicholas Wilson.</div>
<p style="color: grey;"><i>Updated March 6, 2011, 12:34 a.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;">There was a discussion a few months ago about using composer tags in last.fm scrobbling. The patch did what I wanted it to, but after being checked in it was reverted when some people complained. Fair enough. I have got round to adding a config option for this behaviour.
Feedback in particular: I am new to amarok, so might be using the config system incorrectly. It does work though.
Behaviour implemented: when config option is set, scrobble composer preferentially to artist; when not set, always use artist data (default, and same behaviour as before)</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;">It seems to work. Scrobbles as expected with option set and not set, and config is correctly saved between restarts of amarok when set and not set.</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=254580">254580</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>src/amarokconfig.kcfg <span style="color: grey">(1268c33)</span></li>
<li>src/services/lastfm/LastFmConfigWidget.ui <span style="color: grey">(a6e50d3)</span></li>
<li>src/services/lastfm/LastFmService.h <span style="color: grey">(8c30a5d)</span></li>
<li>src/services/lastfm/LastFmService.cpp <span style="color: grey">(b61a8f4)</span></li>
<li>src/services/lastfm/LastFmServiceConfig.h <span style="color: grey">(99b46c2)</span></li>
<li>src/services/lastfm/LastFmServiceConfig.cpp <span style="color: grey">(6ac3ff6)</span></li>
<li>src/services/lastfm/LastFmServiceSettings.cpp <span style="color: grey">(d464c89)</span></li>
<li>src/services/lastfm/ScrobblerAdapter.cpp <span style="color: grey">(940d0c4)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/100806/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>