[rekonq] rekonq 0.1alpha

[Paweł Prażak]

> > > 2.
> > > There are also kde 4.2 derived bugs. (e.g. tab text) I leaved unaltered
> > > code
> > > that works with KDE trunk, but not (well) with 4.2
> >
> > I think it would be better to leave it as we (me and avaddon) coded it,
> > because otherwise you'll get a bug on 4.2 or 4.3.
>
> On kde/trunk these bugs are not present. So it's just a 4.2 related
> problem.
> I cannot understand what code you are referring. I said I did no changes
> about.



    connect(m_tabBar, SIGNAL(closeRequest(int)), this,
SLOT(closeTab(int)));
    bool oneCloseButton =
ReKonfig::showCloseTabButton();

+#if
KDE_IS_VERSION(4,2,60)

+    setTabsClosable(oneCloseButton);  // this causes #23 on KDE
4.2
+#else

     setCloseButtonEnabled(oneCloseButton);  // this is deprecated, remove
for KDE >=4.3
+#endif




>
>
> > > 3.
> > > new download system and modifications in the Application class to
> better
> > > support it. QPointer(s) are really a great idea :)
> > > I also modified a bit download class and removed QTimer notification.
> > > It's my
> > > fault. If someone is interested in, he can re-enable that.
> >
> > Timer is in place to make download progress bar possible.
> > QPointers helped to track the deletion ofo objects when going to
> background
> > download mode and back.
> >
> > > 4.
> > > Side Panel. As requested rekonq 0.1 will have it. We sure have to work
> > > about
> > > the bookmarks panel (before releasing 0.1) and put it on.
> > >
> > > 5.
> > > Bookmark classes improvements. All apart from one method I didn't
> > > understand
> > > and I never seen used.
> >
> > Some stuff there is in place to make future features possible.
> >
> > > 6.
> > > Contextual menu. Here I did the major changes (in the code). I removed
> > > the unuseful (for me) QMap actions and provided a KActionCollection for
> > > all webActions.
> >
> > We don't really need action collection (and it's heavier than QMap) and
> it
> > makes code cleaner and easier to read. There is no point in changing
> > working code just to use KDE class instead of generic container.
>
> Why you said it's heavier? I changed it just to let it be more readable.
> And
> having in mind (future) "Configure shortcuts" dialog insertion


Let's see how much heavier:

    QObject dummy;
    KActionCollection test1(&dummy);
    QMap<QString, QAction> test2;
    kDebug() << "KActionCollection :" << sizeof(test1);
    kDebug() << "QMap              :" << sizeof(test2);

this code output would be:
KActionCollection : 24
QMap              : 8

So as you can see this means 3x heavier (or 300%)

But the bigger issue is that in your implementation action collection isn't
static, so every instance of WebView creates (in constructor) new
collection. So if you have 30 tabs it's like 24 B x 30, but the worst thing
is that it adds overhead for every web tab creation because you invoke
KActionCollection each time you create a WebView object AND you create
(every time) 8 KActions and initialize them and add to action (just to
destruct them when user closes the tab...) I don't have time to test how
much overhead it addes but I bet it's like 50~100%.

24 bytes per tab isn't that bad, but the additional overhead (longer
creation time) is a real problem (especially if you have resource
constrained systems like netbooks).

This collection should be static and lazy (init on demand).


> > 7.
> >
> > > moveable tabs, that is gratis, via Qt 4.5 functions.
> > >
> > >
> > > I hope I'm remembering ALL changes I did. Please from now on, sync one
> > > your branch (master?) with mainline/master, test, code, suggest, do
> small
> > > themable
> > > commits and fill merge requests ;).
> > > I'd like to release 0.1alpha on next tuesday (28 apr) and from there on
> > > going
> > > in bugfixing and translation mode until 0.1 release (when? I don't
> know).
> > >
> > > From now on, if you don't have different ideas, we need to work on two
> > > essential aspects:
> > > - the bookmarks sidepanel
> > > - the focus problem
> > >
> > > About focus, I really cannot decide what is better to do, so I'm
> waiting
> > > for
> > > your suggestions (and your code ;) ).
> >
> > What's with the focus?
> >
> > > COPYRIGHT QUESTION
> > > I added a string on every rekonq file: the following
> > >
> > > * Copyright (C) 2009 rekonq team. Please, see AUTHORS file for details
> *
> > >
> > > This means we need to do also two things:
> > >
> > > 1. update Authors file (important)
> > >        here I wrote 3 columns: name, mail, role
> > >        You just need to add yours and perform a merge request.
> >
> > I'm not sure about the role field, we don't have any roles, so it would
> be
> > a bit artifitial.
>
> You can just indicate there if you are a developer, an artist, a
> translator,
> etc..


OK, now I get it.


>
>
> > 2. update main file (VERY important)
> >
> > >        here you need to add... I'm pushing one example commented in
> > > main.cpp file.
> > >
> > > We can obviously remove the previous line and push all our names also
> on
> > > every
> > > file. I did this way just for convenience. Decide all together one way
> > > and just
> > > do it!!
> >
> > IMO this is bad idea. It's not how things are done. We can have AUTHORS
> > file but leave the copyrights on per file basis.
>
> About me it's ok, as said. I'll remove "rekonq team" string and search for
> copyrights... ;)
>
> > I'd like also rise a concern about backporting changes. It doesn't make
> > sense, because it's easier to port your changes to our fork than vice
> versa
> > and you shouldn't port (especially don't change) code that someone write
> > and tested because you don't know why it was written this way and not
> > another and the possibility of introducing new bug is big. I've run a
> diff
> > and I see there are awful lot of things you didn'd ported and most of
> this
> > is important stuff related to bugfixes and missing features. A lots pasts
> > of code were revritten or heavy refactored and there is no sense in doing
> > it again.
>
> I really find difficult answering here.
> Why if I write something and you modify it, this is an improvement and if I
> change something backporting your code there are risks and possibility of
> introducing new bugs?


It's just the fact that avaddon an I've spend  couple of weeks tweaking the
code and testing, peer-reviewing it, so this isn't something you want to
change if you don't want to introduce new bugs. There are some subtle
changes that have big impact on code quality.

Why your changes are critical or bugfix related and mine are "I didn't
> understand so I removed it"?


And there is more code changed/added. It's numbers not egos that counts.

I asked with first email about features and changes I didn't port. I never
> received patches via mail or merge requests apart from avaddon's fixes to
> localization system (added) and your infamous "I did a lot of things but I
> cannot remember what".


Here are your patches:

$ kompare avaddon-clone/ mainline/
$ kompare pawelprazak-clone/ mainline/

Sorry but I don't have much time lately (there are things that need to be
get sorted, some personal stuff), fixes are in our code.


>
> > Also IMO you can't make a release without unittesting (and unit tests
> I've
> > implemented are not even adopted in mainline yet), to make it right we
> need
> > quite a few new tests to make sure the code can be released to anyone
> (even
> > thou this is a preview).
>
> I usually use (and see used) the shortcut "IMHO". but probably YOUR opinion
> is
> not humble..


Don't be picky ;)

(hint: search Google for phrase: "imo acronym")

I guess there are lots of other things you've never seen...
Why to be afraid to have not-so-humble opinion anyway?

btw. it's one letter less to write ;P

Regards
Paweł
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/rekonq/attachments/20090423/ef558483/attachment.htm