----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119659/#review74165 -----------------------------------------------------------
The function AmarokLyricsScript::ttplayer_codeFunc has some strange comments. Also, at least this function needs a lot more comments and readable variable names to be maintainable. - Ralf Engels On Aug. 8, 2014, 10:04 vorm., Vedant Agarwala wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/119659/ > ----------------------------------------------------------- > > (Updated Aug. 8, 2014, 10:04 vorm.) > > > Review request for Amarok and Mark Kretschmann. > > > Repository: amarok > > > Description > ------- > > I have added an amarok script to download timestamped lyrics. > > > Diffs > ----- > > src/scripting/scriptengine/AmarokLyricsScript.h 9711038 > src/scripting/scriptengine/AmarokLyricsScript.cpp de378dc > src/scripting/scripts/CMakeLists.txt 05ae586 > src/scripting/scripts/lrc_ttplayer/CMakeLists.txt PRE-CREATION > src/scripting/scripts/lrc_ttplayer/main.js PRE-CREATION > src/scripting/scripts/lrc_ttplayer/script.desktop PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/119659/diff/ > > > Testing > ------- > > Yes. > > To test, use the diff uploaded with the request or checkout my personal > branch: > http://quickgit.kde.org/?p=clones/amarok/vedanta/sok.git&a=commit&h=c96f68db858b60a15eef47ddc7faad75ad792501 > > The database (as it turns out) is quite small. (More scripts for the same > need to be added in future) It works for the following songs: > 1. Toxicity - System of a Down > 2. Rockstar - Nickelback > > You will have to "reload lyrics", if you already have lyrics for that song. > > > File Attachments > ---------------- > > diff to use for testing > > https://git.reviewboard.kde.org/media/uploaded/files/2014/08/08/c0d77a2e-462b-4e3f-be70-8d3aa6762ad7__gsocFullDiff_v1.patch > > > Thanks, > > Vedant Agarwala > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel