> On 2010-10-31 20:14:08, Alex Merry wrote:
> > src/EngineController.h, line 117
> > <http://git.reviewboard.kde.org/r/100120/diff/1/?file=2895#file2895line117>
> >
> >     Delete stuff rather than commenting it :-)
> 
> Ralf Engels wrote:
>     It was unused (after my refactoring) and redundant as you can get the 
> state from the media object.

But that doesn't change my comment - you should delete the code if it's not 
needed.  It can always be reclaimed from the git history.


- Alex


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/100120/#review239
-----------------------------------------------------------


On 2010-10-31 15:53:10, Ralf Engels wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100120/
> -----------------------------------------------------------
> 
> (Updated 2010-10-31 15:53:10)
> 
> 
> Review request for Amarok.
> 
> 
> Summary
> -------
> 
> Remove EngineObserver and the observer pattern.
> Instead we have several new signals and functions in EngineController.
> Also the EngineController is now a MetaObserver for the current track and 
> exporting the relevant signals.
> 
> This patch improves the tread safety and reduces the complexity of the code 
> by a lot.
> Over 600 lines of codes could be removed while at the same time fixing the 
> image update problems and several small problems where classes listened for 
> the album meta changed but forgot that the album could change when a user 
> changed the track.
> 
> 
> Diffs
> -----
> 
>   playground/src/context/applets/coverbling/CoverBlingApplet.h a3ff113 
>   playground/src/context/applets/coverbling/CoverBlingApplet.cpp a2a50e7 
>   playground/src/context/applets/video/Video.h 1d52f0b 
>   playground/src/context/applets/video/Video.cpp 8471497 
>   src/ActionClasses.h aa67ec7 
>   src/ActionClasses.cpp 61e8af8 
>   src/App.cpp e6006b9 
>   src/EngineController.h aae4503 
>   src/EngineController.cpp 65b3f2e 
>   src/KNotificationBackend.h 02cc9d8 
>   src/KNotificationBackend.cpp b98391c 
>   src/MainWindow.h 4593d47 
>   src/MainWindow.cpp 9102558 
>   src/TrayIcon.h d1ffd43 
>   src/TrayIcon.cpp 47f56bf 
>   src/amarokurls/AmarokUrlHandler.cpp 1d8cd46 
>   src/context/ContextView.h a3f36fb 
>   src/context/ContextView.cpp ccfe4f3 
>   src/context/applets/photos/PhotosApplet.h 4cac9ea 
>   src/context/applets/photos/PhotosApplet.cpp 0b47da9 
>   src/context/applets/similarartists/SimilarArtistsApplet.h 3a2d7ea 
>   src/context/applets/similarartists/SimilarArtistsApplet.cpp 0cf2c76 
>   src/context/applets/videoclip/VideoclipApplet.h a0704c6 
>   src/context/applets/videoclip/VideoclipApplet.cpp 789173e 
>   src/context/engines/current/CurrentEngine.h 0369065 
>   src/context/engines/current/CurrentEngine.cpp 2c32f75 
>   src/context/engines/labels/LabelsEngine.h 626a030 
>   src/context/engines/labels/LabelsEngine.cpp 65278b4 
>   src/context/engines/lyrics/LyricsEngine.h 771bf6f 
>   src/context/engines/lyrics/LyricsEngine.cpp 527cd0d 
>   src/context/engines/similarartists/SimilarArtistsEngine.h fea9bf5 
>   src/context/engines/similarartists/SimilarArtistsEngine.cpp 97bc109 
>   src/context/engines/upcomingevents/UpcomingEventsEngine.h c57b004 
>   src/context/engines/upcomingevents/UpcomingEventsEngine.cpp 66131fd 
>   src/context/engines/wikipedia/WikipediaEngine.h 5d53082 
>   src/context/engines/wikipedia/WikipediaEngine.cpp 4c65799 
>   src/core-impl/meta/stream/Stream_p.h cd217bf 
>   src/core-impl/meta/timecode/TimecodeObserver.h b727196 
>   src/core-impl/meta/timecode/TimecodeObserver.cpp dea809d 
>   src/core/CMakeLists.txt 5863ca1 
>   src/core/engine/EngineObserver.h 5a93062 
>   src/core/engine/EngineObserver.cpp 7d5728b 
>   src/dbus/mpris1/PlayerHandler.h 8f90f27 
>   src/dbus/mpris1/PlayerHandler.cpp 0afeb59 
>   src/dbus/mpris2/Mpris2DBusHandler.h a767c79 
>   src/dbus/mpris2/Mpris2DBusHandler.cpp 59afbf3 
>   src/dialogs/ScriptManager.h 6104dcf 
>   src/dialogs/ScriptManager.cpp e98f45f 
>   src/dynamic/BiasSolver.cpp bf8e3c8 
>   src/dynamic/biases/EchoNest.h 9a6ecc7 
>   src/dynamic/biases/EchoNest.cpp 16f26f4 
>   src/mac/GrowlInterface.h 3bb35d2 
>   src/mac/GrowlInterface.cpp 862d019 
>   src/playlist/PlaylistActions.h 035ff77 
>   src/playlist/PlaylistActions.cpp 2358b29 
>   src/playlist/PlaylistController.cpp d38c9d6 
>   src/playlist/view/listview/PrettyItemDelegate.cpp ab19ccc 
>   src/scriptengine/AmarokEngineScript.h 55e275c 
>   src/scriptengine/AmarokEngineScript.cpp 15f7b6e 
>   src/services/lastfm/ScrobblerAdapter.h ef7a7c1 
>   src/services/lastfm/ScrobblerAdapter.cpp 1b4da4a 
>   src/services/lastfm/biases/LastFmBias.h 6ba571d 
>   src/services/lastfm/biases/LastFmBias.cpp 4093104 
>   src/statemanagement/DefaultApplicationController.cpp 8205aab 
>   src/statusbar/StatusBar.h b785714 
>   src/statusbar/StatusBar.cpp 48df821 
>   src/toolbar/CurrentTrackToolbar.h c8293f3 
>   src/toolbar/CurrentTrackToolbar.cpp 22408ea 
>   src/toolbar/MainToolbar.h fc29ba2 
>   src/toolbar/MainToolbar.cpp abeedc6 
>   src/toolbar/SlimToolbar.h dcfaf22 
>   src/toolbar/SlimToolbar.cpp e69df7c 
>   src/toolbar/VolumePopupButton.h a0f5d56 
>   src/toolbar/VolumePopupButton.cpp fb5fcb1 
>   src/widgets/Osd.h b80b37e 
>   src/widgets/Osd.cpp af4b24b 
>   src/widgets/ProgressWidget.h 3fba490 
>   src/widgets/ProgressWidget.cpp 211cea7 
>   src/widgets/VolumeWidget.h 910e9f1 
>   src/widgets/VolumeWidget.cpp e1d01f1 
>   tests/core-impl/meta/multi/CMakeLists.txt 8a51249 
>   tests/playlist/CMakeLists.txt 01703fe 
> 
> Diff: http://git.reviewboard.kde.org/r/100120/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ralf
> 
>

_______________________________________________
Amarok-devel mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/amarok-devel

Reply via email to