Re: Review Request 110934: Adds a Offline Mode to Amarok i.e. fixes Bug 229111
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110934/ --- (Updated July 2, 2013, 10:29 a.m.) Status -- This change has been discarded. Review request for Amarok. Description --- 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. This addresses bug 229111. https://bugs.kde.org/show_bug.cgi?id=229111 Diffs - src/ActionClasses.h 0187402 src/ActionClasses.h 0187402 src/ActionClasses.h 0187402 src/ActionClasses.cpp cce6f20 src/ActionClasses.cpp cce6f20 src/ActionClasses.cpp cce6f20 src/MainWindow.cpp af47db7 src/MainWindow.cpp af47db7 src/MainWindow.cpp af47db7 src/amarokconfig.kcfg e00325d src/amarokconfig.kcfg e00325d src/amarokconfig.kcfg e00325d src/context/applets/photos/PhotosScrollWidget.cpp b5e872a src/context/applets/photos/PhotosScrollWidget.cpp b5e872a src/context/applets/similarartists/ArtistWidget.cpp 511f641 src/context/applets/similarartists/ArtistWidget.cpp 511f641 src/context/applets/upcomingevents/UpcomingEventsApplet.cpp 38742df src/context/applets/upcomingevents/UpcomingEventsApplet.cpp 38742df src/context/applets/upcomingevents/UpcomingEventsMapWidget.h 99e55b2 src/context/applets/upcomingevents/UpcomingEventsMapWidget.h 99e55b2 src/context/applets/upcomingevents/UpcomingEventsMapWidget.cpp 9c11506 src/context/applets/upcomingevents/UpcomingEventsMapWidget.cpp 9c11506 src/context/applets/upcomingevents/UpcomingEventsWidget.cpp 323ec1e src/context/applets/upcomingevents/UpcomingEventsWidget.cpp 323ec1e src/context/applets/wikipedia/WikipediaApplet.cpp 0f7a3b0 src/context/applets/wikipedia/WikipediaApplet.cpp 0f7a3b0 src/context/engines/labels/LabelsEngine.cpp ad3ae71 src/context/engines/labels/LabelsEngine.cpp ad3ae71 src/context/engines/photos/PhotosEngine.cpp f9f8fd5 src/context/engines/photos/PhotosEngine.cpp f9f8fd5 src/context/engines/similarartists/SimilarArtistsEngine.cpp 898beb4 src/context/engines/similarartists/SimilarArtistsEngine.cpp 898beb4 src/context/engines/tabs/TabsEngine.cpp 24a89df src/context/engines/tabs/TabsEngine.cpp 24a89df src/context/engines/upcomingevents/UpcomingEventsEngine.cpp b869a3c src/context/engines/upcomingevents/UpcomingEventsEngine.cpp b869a3c src/context/engines/wikipedia/WikipediaEngine.cpp 0b75f15 src/context/engines/wikipedia/WikipediaEngine.cpp 0b75f15 src/context/widgets/DropPixmapItem.cpp 62b0e90 src/context/widgets/DropPixmapItem.cpp 62b0e90 src/covermanager/CoverFetcher.cpp 3cf32cc src/covermanager/CoverFetcher.cpp 3cf32cc src/covermanager/CoverFoundDialog.cpp 6d7efa2 src/covermanager/CoverFoundDialog.cpp 6d7efa2 src/musicbrainz/MusicBrainzFinder.cpp 6892232 src/musicbrainz/MusicBrainzFinder.cpp 6892232 src/musicbrainz/MusicDNSFinder.cpp d393211 src/musicbrainz/MusicDNSFinder.cpp d393211 src/network/NetworkAccessManagerProxy.h 6285f0f src/network/NetworkAccessManagerProxy.h 6285f0f src/network/NetworkAccessManagerProxy.h 6285f0f src/network/NetworkAccessManagerProxy.cpp a2694bf src/network/NetworkAccessManagerProxy.cpp a2694bf src/network/NetworkAccessManagerProxy.cpp a2694bf src/scriptengine/AmarokNetworkScript.cpp 0eaa440 src/scriptengine/AmarokNetworkScript.cpp 0eaa440 src/services/ampache/AmpacheAccountLogin.cpp 26aebed src/services/ampache/AmpacheAccountLogin.cpp 26aebed src/services/ampache/AmpacheServiceCollection.cpp 140c465 src/services/ampache/AmpacheServiceCollection.cpp 140c465 src/services/ampache/AmpacheServiceQueryMaker.cpp b94c3b0 src/services/ampache/AmpacheServiceQueryMaker.cpp b94c3b0 src/services/lastfm/AvatarDownloader.cpp b54b252 src/services/lastfm/AvatarDownloader.cpp b54b252 src/services/lastfm/LastFmService.cpp 4c100c4 src/services/lastfm/LastFmService.cpp 4c100c4 src/services/lastfm/LastFmServiceSettings.h 82b9b00 src/services/lastfm/LastFmServiceSettings.h 82b9b00 src/services/lastfm/LastFmServiceSettings.cpp 086997e src/services/lastfm/LastFmServiceSettings.cpp 086997e Diff: http://git.reviewboard.kde.org/r/110934/diff/ Testing --- Testing done. Builds and runs. I even made sure error messages were being shown (and no other undesired
Re: Review Request 110934: Adds a Offline Mode to Amarok i.e. fixes Bug 229111
On June 11, 2013, 9:26 a.m., Myriam Schweingruber wrote: 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. Vedant Agarwala wrote: 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? Myriam Schweingruber wrote: 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. 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. - Vedant --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110934/#review34125 --- On June 10, 2013, 7:04 p.m., Vedant Agarwala wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110934/ --- (Updated June 10, 2013, 7:04 p.m.) Review request for Amarok. Description --- 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. This addresses bug 229111. https://bugs.kde.org/show_bug.cgi?id=229111 Diffs - src/ActionClasses.h 0187402 src/ActionClasses.h 0187402 src/ActionClasses.h 0187402 src/ActionClasses.cpp cce6f20 src/ActionClasses.cpp cce6f20 src/ActionClasses.cpp cce6f20 src/MainWindow.cpp af47db7 src/MainWindow.cpp af47db7 src/MainWindow.cpp af47db7 src/amarokconfig.kcfg e00325d src/amarokconfig.kcfg e00325d src/amarokconfig.kcfg e00325d src/context/applets/photos/PhotosScrollWidget.cpp b5e872a src/context/applets/photos/PhotosScrollWidget.cpp b5e872a src/context/applets/similarartists/ArtistWidget.cpp 511f641 src/context/applets/similarartists/ArtistWidget.cpp 511f641 src/context/applets/upcomingevents/UpcomingEventsApplet.cpp 38742df src/context/applets/upcomingevents/UpcomingEventsApplet.cpp 38742df src/context/applets/upcomingevents/UpcomingEventsMapWidget.h 99e55b2 src/context/applets/upcomingevents/UpcomingEventsMapWidget.h 99e55b2 src/context/applets/upcomingevents/UpcomingEventsMapWidget.cpp 9c11506 src/context/applets/upcomingevents/UpcomingEventsMapWidget.cpp 9c11506 src/context/applets/upcomingevents/UpcomingEventsWidget.cpp 323ec1e src/context/applets/upcomingevents/UpcomingEventsWidget.cpp 323ec1e src/context/applets/wikipedia/WikipediaApplet.cpp 0f7a3b0
Re: Review Request 110934: Adds a Offline Mode to Amarok i.e. fixes Bug 229111
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110934/#review34125 --- 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. - Myriam Schweingruber On June 10, 2013, 9:04 p.m., Vedant Agarwala wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110934/ --- (Updated June 10, 2013, 9:04 p.m.) Review request for Amarok. Description --- 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. This addresses bug 229111. https://bugs.kde.org/show_bug.cgi?id=229111 Diffs - src/ActionClasses.h 0187402 src/ActionClasses.h 0187402 src/ActionClasses.h 0187402 src/ActionClasses.cpp cce6f20 src/ActionClasses.cpp cce6f20 src/ActionClasses.cpp cce6f20 src/MainWindow.cpp af47db7 src/MainWindow.cpp af47db7 src/MainWindow.cpp af47db7 src/amarokconfig.kcfg e00325d src/amarokconfig.kcfg e00325d src/amarokconfig.kcfg e00325d src/context/applets/photos/PhotosScrollWidget.cpp b5e872a src/context/applets/photos/PhotosScrollWidget.cpp b5e872a src/context/applets/similarartists/ArtistWidget.cpp 511f641 src/context/applets/similarartists/ArtistWidget.cpp 511f641 src/context/applets/upcomingevents/UpcomingEventsApplet.cpp 38742df src/context/applets/upcomingevents/UpcomingEventsApplet.cpp 38742df src/context/applets/upcomingevents/UpcomingEventsMapWidget.h 99e55b2 src/context/applets/upcomingevents/UpcomingEventsMapWidget.h 99e55b2 src/context/applets/upcomingevents/UpcomingEventsMapWidget.cpp 9c11506 src/context/applets/upcomingevents/UpcomingEventsMapWidget.cpp 9c11506 src/context/applets/upcomingevents/UpcomingEventsWidget.cpp 323ec1e src/context/applets/upcomingevents/UpcomingEventsWidget.cpp 323ec1e src/context/applets/wikipedia/WikipediaApplet.cpp 0f7a3b0 src/context/applets/wikipedia/WikipediaApplet.cpp 0f7a3b0 src/context/engines/labels/LabelsEngine.cpp ad3ae71 src/context/engines/labels/LabelsEngine.cpp ad3ae71 src/context/engines/photos/PhotosEngine.cpp f9f8fd5 src/context/engines/photos/PhotosEngine.cpp f9f8fd5 src/context/engines/similarartists/SimilarArtistsEngine.cpp 898beb4 src/context/engines/similarartists/SimilarArtistsEngine.cpp 898beb4 src/context/engines/tabs/TabsEngine.cpp 24a89df src/context/engines/tabs/TabsEngine.cpp 24a89df src/context/engines/upcomingevents/UpcomingEventsEngine.cpp b869a3c src/context/engines/upcomingevents/UpcomingEventsEngine.cpp b869a3c src/context/engines/wikipedia/WikipediaEngine.cpp 0b75f15 src/context/engines/wikipedia/WikipediaEngine.cpp 0b75f15 src/context/widgets/DropPixmapItem.cpp 62b0e90 src/context/widgets/DropPixmapItem.cpp 62b0e90 src/covermanager/CoverFetcher.cpp 3cf32cc src/covermanager/CoverFetcher.cpp 3cf32cc src/covermanager/CoverFoundDialog.cpp 6d7efa2 src/covermanager/CoverFoundDialog.cpp 6d7efa2 src/musicbrainz/MusicBrainzFinder.cpp 6892232 src/musicbrainz/MusicBrainzFinder.cpp 6892232 src/musicbrainz/MusicDNSFinder.cpp d393211 src/musicbrainz/MusicDNSFinder.cpp d393211 src/network/NetworkAccessManagerProxy.h 6285f0f src/network/NetworkAccessManagerProxy.h 6285f0f src/network/NetworkAccessManagerProxy.h 6285f0f src/network/NetworkAccessManagerProxy.cpp a2694bf src/network/NetworkAccessManagerProxy.cpp a2694bf src/network/NetworkAccessManagerProxy.cpp a2694bf src/scriptengine/AmarokNetworkScript.cpp 0eaa440 src/scriptengine/AmarokNetworkScript.cpp 0eaa440 src/services/ampache/AmpacheAccountLogin.cpp 26aebed src/services/ampache/AmpacheAccountLogin.cpp 26aebed src/services/ampache/AmpacheServiceCollection.cpp 140c465 src/services/ampache/AmpacheServiceCollection.cpp 140c465 src/services/ampache/AmpacheServiceQueryMaker.cpp b94c3b0 src/services/ampache/AmpacheServiceQueryMaker.cpp b94c3b0
Review Request 110934: Adds a Offline Mode to Amarok i.e. fixes Bug 229111
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110934/ --- Review request for Amarok. Summary (updated) - Adds a Offline Mode to Amarok i.e. fixes Bug 229111 Description (updated) --- 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. This addresses bug 229111. https://bugs.kde.org/show_bug.cgi?id=229111 Diffs (updated) - src/ActionClasses.h 0187402 src/ActionClasses.h 0187402 src/ActionClasses.h 0187402 src/ActionClasses.cpp cce6f20 src/ActionClasses.cpp cce6f20 src/ActionClasses.cpp cce6f20 src/MainWindow.cpp af47db7 src/MainWindow.cpp af47db7 src/MainWindow.cpp af47db7 src/amarokconfig.kcfg e00325d src/amarokconfig.kcfg e00325d src/amarokconfig.kcfg e00325d src/context/applets/photos/PhotosScrollWidget.cpp b5e872a src/context/applets/photos/PhotosScrollWidget.cpp b5e872a src/context/applets/similarartists/ArtistWidget.cpp 511f641 src/context/applets/similarartists/ArtistWidget.cpp 511f641 src/context/applets/upcomingevents/UpcomingEventsApplet.cpp 38742df src/context/applets/upcomingevents/UpcomingEventsApplet.cpp 38742df src/context/applets/upcomingevents/UpcomingEventsMapWidget.h 99e55b2 src/context/applets/upcomingevents/UpcomingEventsMapWidget.h 99e55b2 src/context/applets/upcomingevents/UpcomingEventsMapWidget.cpp 9c11506 src/context/applets/upcomingevents/UpcomingEventsMapWidget.cpp 9c11506 src/context/applets/upcomingevents/UpcomingEventsWidget.cpp 323ec1e src/context/applets/upcomingevents/UpcomingEventsWidget.cpp 323ec1e src/context/applets/wikipedia/WikipediaApplet.cpp 0f7a3b0 src/context/applets/wikipedia/WikipediaApplet.cpp 0f7a3b0 src/context/engines/labels/LabelsEngine.cpp ad3ae71 src/context/engines/labels/LabelsEngine.cpp ad3ae71 src/context/engines/photos/PhotosEngine.cpp f9f8fd5 src/context/engines/photos/PhotosEngine.cpp f9f8fd5 src/context/engines/similarartists/SimilarArtistsEngine.cpp 898beb4 src/context/engines/similarartists/SimilarArtistsEngine.cpp 898beb4 src/context/engines/tabs/TabsEngine.cpp 24a89df src/context/engines/tabs/TabsEngine.cpp 24a89df src/context/engines/upcomingevents/UpcomingEventsEngine.cpp b869a3c src/context/engines/upcomingevents/UpcomingEventsEngine.cpp b869a3c src/context/engines/wikipedia/WikipediaEngine.cpp 0b75f15 src/context/engines/wikipedia/WikipediaEngine.cpp 0b75f15 src/context/widgets/DropPixmapItem.cpp 62b0e90 src/context/widgets/DropPixmapItem.cpp 62b0e90 src/covermanager/CoverFetcher.cpp 3cf32cc src/covermanager/CoverFetcher.cpp 3cf32cc src/covermanager/CoverFoundDialog.cpp 6d7efa2 src/covermanager/CoverFoundDialog.cpp 6d7efa2 src/musicbrainz/MusicBrainzFinder.cpp 6892232 src/musicbrainz/MusicBrainzFinder.cpp 6892232 src/musicbrainz/MusicDNSFinder.cpp d393211 src/musicbrainz/MusicDNSFinder.cpp d393211 src/network/NetworkAccessManagerProxy.h 6285f0f src/network/NetworkAccessManagerProxy.h 6285f0f src/network/NetworkAccessManagerProxy.h 6285f0f src/network/NetworkAccessManagerProxy.cpp a2694bf src/network/NetworkAccessManagerProxy.cpp a2694bf src/network/NetworkAccessManagerProxy.cpp a2694bf src/scriptengine/AmarokNetworkScript.cpp 0eaa440 src/scriptengine/AmarokNetworkScript.cpp 0eaa440 src/services/ampache/AmpacheAccountLogin.cpp 26aebed src/services/ampache/AmpacheAccountLogin.cpp 26aebed src/services/ampache/AmpacheServiceCollection.cpp 140c465 src/services/ampache/AmpacheServiceCollection.cpp 140c465 src/services/ampache/AmpacheServiceQueryMaker.cpp b94c3b0 src/services/ampache/AmpacheServiceQueryMaker.cpp b94c3b0 src/services/lastfm/AvatarDownloader.cpp b54b252 src/services/lastfm/AvatarDownloader.cpp b54b252 src/services/lastfm/LastFmService.cpp 4c100c4 src/services/lastfm/LastFmService.cpp 4c100c4 src/services/lastfm/LastFmServiceSettings.h 82b9b00 src/services/lastfm/LastFmServiceSettings.h 82b9b00 src/services/lastfm/LastFmServiceSettings.cpp 086997e src/services/lastfm/LastFmServiceSettings.cpp 086997e Diff: http://git.reviewboard.kde.org/r/110934/diff/ Testing (updated) --- Testing done. Builds and runs. I even made sure error messages were being
