----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114765/#review46684 -----------------------------------------------------------
Hi, thanks for the patch. I think there could be a better approach in solving the bug - the one I've outlined in my review of a similar request: https://git.reviewboard.kde.org/r/113057/ It may turn out not possible/worth it, but it is for sure worth investigation. src/playlist/layouts/PlaylistLayoutEditDialog.cpp <https://git.reviewboard.kde.org/r/114765/#comment33326> Please respect Amarok coding style - spaces around arguments - see the HACKING folder in Amarok sources. This applies to other places in your patch. Note that existing calls buttonBox->button(QDialogButtonBox::Apply) do not conform to Amarok coding style - you may fix that in a separate review request (not in this one as we want style fixes separate from other fixes) - Matěj Laitl On Dec. 31, 2013, 6:04 p.m., Nilesh Suthar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/114765/ > ----------------------------------------------------------- > > (Updated Dec. 31, 2013, 6:04 p.m.) > > > Review request for Amarok. > > > Bugs: 322016 > https://bugs.kde.org/show_bug.cgi?id=322016 > > > Repository: amarok > > > Description > ------- > > Disabled Apply Button on Editor Creation and Close.on any change the apply > button is enabled. > > > Diffs > ----- > > src/playlist/layouts/PlaylistLayoutEditDialog.cpp 99aee2a > > Diff: https://git.reviewboard.kde.org/r/114765/diff/ > > > Testing > ------- > > > Thanks, > > Nilesh Suthar > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel