----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109470/#review29229 -----------------------------------------------------------
While we wait for the active developers to decide if this feature is going to be introduced, there are a few formatting issues to be addressed before we proceed. As a rule, before you submit a patch, always make sure it adheres to the coding style laid out in HACKING/intro_and_style.txt And don't let this bother you too much, almost everyone has coding style issues in their first patch. src/context/applets/lyrics/LyricsApplet.cpp <http://git.reviewboard.kde.org/r/109470/#comment21837> The defines and includes shouldn't be clubbed together. Let the newline remain here. src/context/applets/lyrics/LyricsApplet.cpp <http://git.reviewboard.kde.org/r/109470/#comment21838> No need for a newline here. src/context/applets/lyrics/LyricsApplet.cpp <http://git.reviewboard.kde.org/r/109470/#comment21843> Again, no need for the newlines. Keep the coding style consistent. src/context/applets/lyrics/LyricsApplet.cpp <http://git.reviewboard.kde.org/r/109470/#comment21840> Leading/Trailing whitespaces to be removed. You might not be aware of their presence in the text editor/IDE you're using. So make sure the indentation is done using spaces and not tabs. If you're using Kate, you can do this by going to Settings->Configure Kate->Editor Component->Editing->Indentation and setting 'Indent using' to 'Spaces'. You could also use Mark's Vim configuration present in HACKING/intro_and_style.txt if you are using Vim. src/context/applets/lyrics/LyricsApplet.cpp <http://git.reviewboard.kde.org/r/109470/#comment21844> Correct the formatting issues here. Refer to the 'Formatting' section of HACKING/intro_and_style to correct the issues : spaces between brackets and arguments, trailing whitespaces, indentation, etc. src/context/applets/lyrics/LyricsApplet.cpp <http://git.reviewboard.kde.org/r/109470/#comment21845> Trailing whitespace. src/context/applets/lyrics/LyricsApplet.cpp <http://git.reviewboard.kde.org/r/109470/#comment21846> Unnecessary newlines. src/context/applets/lyrics/LyricsApplet.cpp <http://git.reviewboard.kde.org/r/109470/#comment21847> Here too. src/context/applets/lyrics/LyricsApplet.cpp <http://git.reviewboard.kde.org/r/109470/#comment21848> Indentation issue. src/context/applets/lyrics/LyricsApplet.cpp <http://git.reviewboard.kde.org/r/109470/#comment21849> Superfluous newline. src/context/engines/lyrics/LyricsEngine.h <http://git.reviewboard.kde.org/r/109470/#comment21850> Indentation issue. src/context/engines/lyrics/LyricsEngine.cpp <http://git.reviewboard.kde.org/r/109470/#comment21851> As mentioned previously, defines and includes are not to be clubbed together. src/context/engines/lyrics/LyricsEngine.cpp <http://git.reviewboard.kde.org/r/109470/#comment21852> Trailing whitespace. src/context/engines/lyrics/LyricsEngine.cpp <http://git.reviewboard.kde.org/r/109470/#comment21856> Spaces needed between brackets and argument : newCacheLyrics( lyrics ) src/context/engines/lyrics/LyricsEngine.cpp <http://git.reviewboard.kde.org/r/109470/#comment21853> Superfluous newline. src/context/engines/lyrics/LyricsEngine.cpp <http://git.reviewboard.kde.org/r/109470/#comment21854> Indentation issue. src/context/engines/lyrics/LyricsEngine.cpp <http://git.reviewboard.kde.org/r/109470/#comment21855> Spaces needed between brackets and function arguments. src/context/engines/lyrics/LyricsEngine.cpp <http://git.reviewboard.kde.org/r/109470/#comment21857> Let the newline remain here. - Jasneet Bhatti On March 14, 2013, 7:10 p.m., mayank jha wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/109470/ > ----------------------------------------------------------- > > (Updated March 14, 2013, 7:10 p.m.) > > > Review request for Amarok, Samikshan Bairagya, Jasneet Bhatti, Bartosz > Brachaczek, Tirtha Chatterjee, and Matěj Laitl. > > > Description > ------- > > It required modifications, when there is no change in the lyrics downloaded > and lyrics retrieved from cache the title display of the lyrics browser > changes to "Cached Lyrics" from "Lyrics" so we can tell the difference > between old and new. > > > Diffs > ----- > > src/context/applets/lyrics/LyricsApplet.cpp 2394964 > src/context/engines/lyrics/LyricsEngine.h b187b73 > src/context/engines/lyrics/LyricsEngine.cpp 2befa91 > > Diff: http://git.reviewboard.kde.org/r/109470/diff/ > > > Testing > ------- > > Its working fine! > > > Thanks, > > mayank jha > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
