----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100373/#review859 -----------------------------------------------------------
src/mainwindow.cpp <http://git.reviewboard.kde.org/r/100373/#comment642> Why do you convert the data as String? You could use integers with QVariant and avoid all the conversions back and forth. I am also not a fan of this special case at -1. I see that becoming a problem when someone will forget and use that index in a data structure. src/mainwindow.cpp <http://git.reviewboard.kde.org/r/100373/#comment647> Coding style: missing a space between the if and (. And there is one superfluous space between the two parenthesis at the end :) src/mainwindow.cpp <http://git.reviewboard.kde.org/r/100373/#comment649> Since you are using ::exec(), the dialog cannot exist behond this function. So you could just allocated it on the stack. src/mainwindow.cpp <http://git.reviewboard.kde.org/r/100373/#comment648> Does this really need to be modal? src/useragent/useragentinfo.h <http://git.reviewboard.kde.org/r/100373/#comment644> I think this API is more complicated than it should be. I would have UserAgentInfo with the same method but without the integer as argument, and a provider returning a QList of UserAgentInfo. src/useragent/useragentwidget.cpp <http://git.reviewboard.kde.org/r/100373/#comment646> You could just not have a destructor. - Benjamin On Jan. 12, 2011, 10:56 p.m., Andrea Diamantini wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/100373/ > ----------------------------------------------------------- > > (Updated Jan. 12, 2011, 10:56 p.m.) > > > Review request for rekonq. > > > Summary > ------- > > User Agent Switcher. > > This patch should implement the UA switcher ability for rekonq. This is a > first implementation, but it seems working quite well. It is based on KDE UA > management and should be fully compatible with konqueror's one i.e. it should > be possible using alternatively rekonq and konqueror sharing the same setting. > > Anyway, this is a different implementation from the konqueror's one. Simpler > and based on the idea > of a future moving to a plugin. > > > Diffs > ----- > > src/CMakeLists.txt f0310b4d91229a6d8322c163069165b940c41efe > src/application.cpp 95aa9cf0b0a40006c5c0e1663acce506e4c4e123 > src/mainwindow.h 33fd20212b9bb69d9450298e4e43324885f8bdf0 > src/mainwindow.cpp f662d7aa77d8045372c43514c6a04ea497a9afbc > src/useragent/useragentinfo.h PRE-CREATION > src/useragent/useragentinfo.cpp PRE-CREATION > src/useragent/useragentsettings.ui PRE-CREATION > src/useragent/useragentwidget.h PRE-CREATION > src/useragent/useragentwidget.cpp PRE-CREATION > > Diff: http://git.reviewboard.kde.org/r/100373/diff > > > Testing > ------- > > > Thanks, > > Andrea > >
_______________________________________________ rekonq mailing list [email protected] https://mail.kde.org/mailman/listinfo/rekonq
