----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100305/#review885 -----------------------------------------------------------
src/application.cpp <http://git.reviewboard.kde.org/r/100305/#comment700> style fixes could go in a different commit. Could you please split it? src/application.cpp <http://git.reviewboard.kde.org/r/100305/#comment702> style fix (see above) src/application.cpp <http://git.reviewboard.kde.org/r/100305/#comment703> style fix (see above) src/application.cpp <http://git.reviewboard.kde.org/r/100305/#comment704> style fix (see above) src/application.cpp <http://git.reviewboard.kde.org/r/100305/#comment701> maybe a more explicit variable name would help making the code more readable. src/application.cpp <http://git.reviewboard.kde.org/r/100305/#comment705> style fix (see above) src/application.cpp <http://git.reviewboard.kde.org/r/100305/#comment706> style fix (see above) src/application.cpp <http://git.reviewboard.kde.org/r/100305/#comment707> style fix (see above) src/application.cpp <http://git.reviewboard.kde.org/r/100305/#comment708> style fix (see above) src/mainview.h <http://git.reviewboard.kde.org/r/100305/#comment709> why move it ? src/tabbar.cpp <http://git.reviewboard.kde.org/r/100305/#comment710> I'm personally not a big fan of hijacking the tabData thingy for this purpose. Couldn't you keep a private QBitArray member and an accessor isTabPinned(int index) ? src/tabbar.cpp <http://git.reviewboard.kde.org/r/100305/#comment713> the whole concept of using -1 as a default value for the (valid) current index feels a bit weird to me. src/tabbar.cpp <http://git.reviewboard.kde.org/r/100305/#comment712> I'm not sure this case is even worth considering. src/tabbar.cpp <http://git.reviewboard.kde.org/r/100305/#comment711> This could go very wrong. If for any reason indexOf returns -1 (for instance you're not looking at the same email in your pinned GMail), then I fear the whole thing may crash ! A check would be slightly better, but overall I feel the persistance could be handled differently. src/tabbar.cpp <http://git.reviewboard.kde.org/r/100305/#comment714> actually, maybe this should respect the aspect ratio of the tab, just like other previews do. (i.e don't hardcode the height) src/tabbar.cpp <http://git.reviewboard.kde.org/r/100305/#comment715> same remark for tabData() - Pierre On Jan. 14, 2011, 2:21 p.m., Furkan Üzümcü wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/100305/ > ----------------------------------------------------------- > > (Updated Jan. 14, 2011, 2:21 p.m.) > > > Review request for rekonq. > > > Summary > ------- > > Added support for pinning tabs like Firefox 4 and Google Chrome. > The default preview size was discussed on IRC. > > > Diffs > ----- > > src/application.cpp 466a0a4 > src/mainview.h bc8b676 > src/mainview.cpp cbe862f > src/rekonq.kcfg cda76d8 > src/tabbar.h 4170786 > src/tabbar.cpp fcc7b78 > > Diff: http://git.reviewboard.kde.org/r/100305/diff > > > Testing > ------- > > * Tested and works. > > > Thanks, > > Furkan > >
_______________________________________________ rekonq mailing list [email protected] https://mail.kde.org/mailman/listinfo/rekonq
