[rekonq] rekonq 0.1alpha

[Andrea Diamantini]

On Thursday 23 April 2009 17:36:36 you wrote:
> > > > 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

I'm having some troubles at work, so I'm having no time to work on rekonq 
these days. Hope on sunday I can do it.
I'm doing a second "tranche" about importing your changes. And I fixed it as 
suggested.

> > > > 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).

Yeah, that's definitely an error. I fixed it.
Anyway, I'd like to insist about using KActionCollection. Don't you think that 
this allows code to be more readable from kde people and allows us to use 
additional KActionCollection features, for free?!
Do you think this is a positive trade-off?

> > > 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ł

Ok, I surely understand your position and your ideas. Anyway my matter is not 
a problem about ego or else. It's just the fact that I started this project 
(and wanna continuing it) for fun and for passion.
So, it's just non sense importing other's code without understanding well its 
meaning and its utility. 
I just demonstrated I can admit my errors or import some unneeded (for me, 
obviously) features.
So, if you (or everyone else) wanna debate with me about code changes, it's ok 
and I'm really glad about. If you wanna impose your changes to my code, I'm 
sorry this is not the case.
I hope all you do the same with your things (and your code).

Regards,

-- 
Andrea Diamantini,
rekonq project
WEB: http://rekonq.sourceforge.net
IRC: adjam_AT_freenode
PGP/GPG : 91A712C1
Fingerprint: 571E DFF4 19EF A597 2CCD A811 6CB6 3538 91A7 12C1