[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-pim
Subject:    Re: [Kde-pim] [patch] Conversion from K3ListView to Qt4 native
From:       Thomas McGuire <Thomas.McGuire () gmx ! net>
Date:       2008-03-30 19:45:50
Message-ID: 200803302145.56705.Thomas.McGuire () gmx ! net
[Download RAW message or body]

[Attachment #2 (multipart/signed)]


Hi,

On Saturday 29 March 2008, Szymon Stefanek wrote:
> I'm planning to apply to GSoC as a student. One of the tasks in my
> application would be helping in the port of the Qt3 compatibility
> widgets to Qt4 native ones in kmail.
>
> I've just given it a first try on a very simple kmail widget.
> The patch against the current trunk is attached.
>
> Can anybody take a look at it, try it out, and possibly commit ?
Thanks for the patch, good work!
I tested it and it worked fine, just a couple of minor points:

- No need for slotAccountListItemClicked(), you can write  
> connect( mAccountList, SIGNAL(itemClicked(QTreeWidgetItem *,int)),
>          this, SLOT(slotApplicableAccountsChanged()) ); 

- Instead of connecting to itemClicked() in the above case, connect to 
  itemChanged(), which will also catch changing the checked state with the
  spacebar
- Don't include qtreewidget.h in the header file, instead forward-declare it 
  like it was done with K3ListView. This speeds up compiling, as other files 
  which include kmfilterdlg.h don't automatically include qtreewidget.h then.
  Only include qtreewidget.h in the cpp file.
- Don't use Hungarion notation, like pHeader. We only use mFoo for member 
  variables, but nothing else.
- Coding style: Yes, the coding style of KDEPIM seems strange at first, but 
  nevertheless it should be consistent. That includes spaces at the inside of 
  parenthesis (so it would be new QTreeWidgetItem( 0 ) instead of new  
  QTreeWidgetItem(0), for example.
  Also, most of the time, the opening brace ({) is put on the same line as the 
  the if/for/whatever statement.

I've committed the patch (with the minor changes above) in revision 791936[1]. 
Again, thanks for the patch.

Regards,
Thomas

[1] http://websvn.kde.org/?view=rev&revision=791936

["signature.asc" (application/pgp-signature)]

_______________________________________________
KDE PIM mailing list kde-pim@kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/

[prev in list] [next in list] [prev in thread] [next in thread]