<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/110934/">http://git.reviewboard.kde.org/r/110934/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 11th, 2013, 9:26 a.m. UTC, <b>Myriam Schweingruber</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;">Vedant, you really need to talk to us on amarok-devel@kde.org or in #amarok before venturing in fixing obscure feature requests. It is highly unlikely this is a desired feature for the Amarok Team, and it should not be solved in Amarok itself.</pre>
</blockquote>
<p>On June 15th, 2013, 11:03 a.m. UTC, <b>Vedant Agarwala</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;">Oh I'm sorry. I didn't know the protocol. I thought it was a confirmed bug and it needed fixing. So this is just some work wasted?</pre>
</blockquote>
<p>On June 15th, 2013, 12:39 p.m. UTC, <b>Myriam Schweingruber</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;">Well, it is a Wish, not a bug, and just if it is requested by two users doesn't make this a valid feature. Everybody can request features, that doesn't make them valid, and if we would implement all open wishes we could as well stop working completely, as many are contradicting each other. In general, we do not confirm wishes, if this was confirmed by another user this is hard to avoid, but it doesn't make it a valid feature.
In general: all new features nee to be discussed on the mailing list before even starting to work on. In particular newcomers should always ask on the mailing list before starting to work on something, as the core team is the one that decides what goes in Amarok and what not.
Sorry if you have wasted time on this, but I remember having told you that the amarok-devel@kde.org mailing list is where everything needs to be discussed, so if you want to avoid losing time on obscure features you really need to discuss this on the mailing list before starting to work on them.</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;">Well the earlier bugs I had fixed, I did not talk about them on the mailing lists. But now that you have mentioned it, I remember commenting on a bug that I wanted to fix, on the KDE bugtracking system. I started working on them only after confirmation. I should've done that here too. Actually I thought that "confirmed" bugs are confirmed by the Amarok team. My bad it seems.
Anyway, this review and another recent one ( https://git.reviewboard.kde.org/r/111038/ ) seem to be the result of the same misunderstanding by me. Now, I
will make sure to confirm before starting any work so that such things don't take place in future.
I want to become a developer for Amarok and with every review I learn more about the code and the community.</pre>
<br />
<p>- Vedant</p>
<br />
<p>On June 10th, 2013, 7:04 p.m. UTC, Vedant Agarwala 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 Vedant Agarwala.</div>
<p style="color: grey;"><i>Updated June 10, 2013, 7:04 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;">Adds a "Offline Mode" to the "Amarok" menu. In Offline Mode, The::networkAccessManager()->getData(...) returns NULL, and prints a error() message.
Also, I have updated existing code so that it checks whether Amarok is in Offline Mode before placing the getData request.
However, this is not a fail safe mechanism. Newer codes can use the QNetworkAccessManager::get(...) direcrtly or make use of the The::networkAccessManager in other ways. Scripts cannot do this since its methods check first but this too can be changed. So, care still has to be taken while writing newer code. I have mentioned the same as comments in the code.</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;">Testing done. Builds and runs. I even made sure error messages were being shown (and no other undesired effects, like a crash, were taking place) if The::networkAccessManager()->getData(...) was called inappropriately.</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=229111">229111</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/ActionClasses.h <span style="color: grey">(0187402)</span></li>
<li>src/ActionClasses.h <span style="color: grey">(0187402)</span></li>
<li>src/ActionClasses.h <span style="color: grey">(0187402)</span></li>
<li>src/ActionClasses.cpp <span style="color: grey">(cce6f20)</span></li>
<li>src/ActionClasses.cpp <span style="color: grey">(cce6f20)</span></li>
<li>src/ActionClasses.cpp <span style="color: grey">(cce6f20)</span></li>
<li>src/MainWindow.cpp <span style="color: grey">(af47db7)</span></li>
<li>src/MainWindow.cpp <span style="color: grey">(af47db7)</span></li>
<li>src/MainWindow.cpp <span style="color: grey">(af47db7)</span></li>
<li>src/amarokconfig.kcfg <span style="color: grey">(e00325d)</span></li>
<li>src/amarokconfig.kcfg <span style="color: grey">(e00325d)</span></li>
<li>src/amarokconfig.kcfg <span style="color: grey">(e00325d)</span></li>
<li>src/context/applets/photos/PhotosScrollWidget.cpp <span style="color: grey">(b5e872a)</span></li>
<li>src/context/applets/photos/PhotosScrollWidget.cpp <span style="color: grey">(b5e872a)</span></li>
<li>src/context/applets/similarartists/ArtistWidget.cpp <span style="color: grey">(511f641)</span></li>
<li>src/context/applets/similarartists/ArtistWidget.cpp <span style="color: grey">(511f641)</span></li>
<li>src/context/applets/upcomingevents/UpcomingEventsApplet.cpp <span style="color: grey">(38742df)</span></li>
<li>src/context/applets/upcomingevents/UpcomingEventsApplet.cpp <span style="color: grey">(38742df)</span></li>
<li>src/context/applets/upcomingevents/UpcomingEventsMapWidget.h <span style="color: grey">(99e55b2)</span></li>
<li>src/context/applets/upcomingevents/UpcomingEventsMapWidget.h <span style="color: grey">(99e55b2)</span></li>
<li>src/context/applets/upcomingevents/UpcomingEventsMapWidget.cpp <span style="color: grey">(9c11506)</span></li>
<li>src/context/applets/upcomingevents/UpcomingEventsMapWidget.cpp <span style="color: grey">(9c11506)</span></li>
<li>src/context/applets/upcomingevents/UpcomingEventsWidget.cpp <span style="color: grey">(323ec1e)</span></li>
<li>src/context/applets/upcomingevents/UpcomingEventsWidget.cpp <span style="color: grey">(323ec1e)</span></li>
<li>src/context/applets/wikipedia/WikipediaApplet.cpp <span style="color: grey">(0f7a3b0)</span></li>
<li>src/context/applets/wikipedia/WikipediaApplet.cpp <span style="color: grey">(0f7a3b0)</span></li>
<li>src/context/engines/labels/LabelsEngine.cpp <span style="color: grey">(ad3ae71)</span></li>
<li>src/context/engines/labels/LabelsEngine.cpp <span style="color: grey">(ad3ae71)</span></li>
<li>src/context/engines/photos/PhotosEngine.cpp <span style="color: grey">(f9f8fd5)</span></li>
<li>src/context/engines/photos/PhotosEngine.cpp <span style="color: grey">(f9f8fd5)</span></li>
<li>src/context/engines/similarartists/SimilarArtistsEngine.cpp <span style="color: grey">(898beb4)</span></li>
<li>src/context/engines/similarartists/SimilarArtistsEngine.cpp <span style="color: grey">(898beb4)</span></li>
<li>src/context/engines/tabs/TabsEngine.cpp <span style="color: grey">(24a89df)</span></li>
<li>src/context/engines/tabs/TabsEngine.cpp <span style="color: grey">(24a89df)</span></li>
<li>src/context/engines/upcomingevents/UpcomingEventsEngine.cpp <span style="color: grey">(b869a3c)</span></li>
<li>src/context/engines/upcomingevents/UpcomingEventsEngine.cpp <span style="color: grey">(b869a3c)</span></li>
<li>src/context/engines/wikipedia/WikipediaEngine.cpp <span style="color: grey">(0b75f15)</span></li>
<li>src/context/engines/wikipedia/WikipediaEngine.cpp <span style="color: grey">(0b75f15)</span></li>
<li>src/context/widgets/DropPixmapItem.cpp <span style="color: grey">(62b0e90)</span></li>
<li>src/context/widgets/DropPixmapItem.cpp <span style="color: grey">(62b0e90)</span></li>
<li>src/covermanager/CoverFetcher.cpp <span style="color: grey">(3cf32cc)</span></li>
<li>src/covermanager/CoverFetcher.cpp <span style="color: grey">(3cf32cc)</span></li>
<li>src/covermanager/CoverFoundDialog.cpp <span style="color: grey">(6d7efa2)</span></li>
<li>src/covermanager/CoverFoundDialog.cpp <span style="color: grey">(6d7efa2)</span></li>
<li>src/musicbrainz/MusicBrainzFinder.cpp <span style="color: grey">(6892232)</span></li>
<li>src/musicbrainz/MusicBrainzFinder.cpp <span style="color: grey">(6892232)</span></li>
<li>src/musicbrainz/MusicDNSFinder.cpp <span style="color: grey">(d393211)</span></li>
<li>src/musicbrainz/MusicDNSFinder.cpp <span style="color: grey">(d393211)</span></li>
<li>src/network/NetworkAccessManagerProxy.h <span style="color: grey">(6285f0f)</span></li>
<li>src/network/NetworkAccessManagerProxy.h <span style="color: grey">(6285f0f)</span></li>
<li>src/network/NetworkAccessManagerProxy.h <span style="color: grey">(6285f0f)</span></li>
<li>src/network/NetworkAccessManagerProxy.cpp <span style="color: grey">(a2694bf)</span></li>
<li>src/network/NetworkAccessManagerProxy.cpp <span style="color: grey">(a2694bf)</span></li>
<li>src/network/NetworkAccessManagerProxy.cpp <span style="color: grey">(a2694bf)</span></li>
<li>src/scriptengine/AmarokNetworkScript.cpp <span style="color: grey">(0eaa440)</span></li>
<li>src/scriptengine/AmarokNetworkScript.cpp <span style="color: grey">(0eaa440)</span></li>
<li>src/services/ampache/AmpacheAccountLogin.cpp <span style="color: grey">(26aebed)</span></li>
<li>src/services/ampache/AmpacheAccountLogin.cpp <span style="color: grey">(26aebed)</span></li>
<li>src/services/ampache/AmpacheServiceCollection.cpp <span style="color: grey">(140c465)</span></li>
<li>src/services/ampache/AmpacheServiceCollection.cpp <span style="color: grey">(140c465)</span></li>
<li>src/services/ampache/AmpacheServiceQueryMaker.cpp <span style="color: grey">(b94c3b0)</span></li>
<li>src/services/ampache/AmpacheServiceQueryMaker.cpp <span style="color: grey">(b94c3b0)</span></li>
<li>src/services/lastfm/AvatarDownloader.cpp <span style="color: grey">(b54b252)</span></li>
<li>src/services/lastfm/AvatarDownloader.cpp <span style="color: grey">(b54b252)</span></li>
<li>src/services/lastfm/LastFmService.cpp <span style="color: grey">(4c100c4)</span></li>
<li>src/services/lastfm/LastFmService.cpp <span style="color: grey">(4c100c4)</span></li>
<li>src/services/lastfm/LastFmServiceSettings.h <span style="color: grey">(82b9b00)</span></li>
<li>src/services/lastfm/LastFmServiceSettings.h <span style="color: grey">(82b9b00)</span></li>
<li>src/services/lastfm/LastFmServiceSettings.cpp <span style="color: grey">(086997e)</span></li>
<li>src/services/lastfm/LastFmServiceSettings.cpp <span style="color: grey">(086997e)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/110934/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>