> 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 > 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 effects, like a crash, were taking place) if > The::networkAccessManager()->getData(...) was called inappropriately. > > > Thanks, > > Vedant Agarwala > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
