> On Jan. 12, 2011, 11:33 p.m., Benjamin Poulain wrote: > > src/mainwindow.cpp, line 1422 > > <http://git.reviewboard.kde.org/r/100373/diff/1/?file=6725#file6725line1422> > > > > 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.
Right about conversion. Fixed. I cannot see the case of using -1 in some data structure. Anyway everything depends on the API choice. Thinking a bit better about it. > On Jan. 12, 2011, 11:33 p.m., Benjamin Poulain wrote: > > src/mainwindow.cpp, lines 1534-1542 > > <http://git.reviewboard.kde.org/r/100373/diff/1/?file=6725#file6725line1534> > > > > Since you are using ::exec(), the dialog cannot exist behond this > > function. So you could just allocated it on the stack. This code comes from here: http://www.kdedevelopers.org/node/3919 > On Jan. 12, 2011, 11:33 p.m., Benjamin Poulain wrote: > > src/useragent/useragentwidget.cpp, lines 59-61 > > <http://git.reviewboard.kde.org/r/100373/diff/1/?file=6730#file6730line59> > > > > You could just not have a destructor. Yeah! Sometimes automatism go beyond needing. Fixed in next patch. - Andrea ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100373/#review859 ----------------------------------------------------------- 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
