[Kmymoney-devel] Review Request: Allan's csvimporter plugin

[Alvaro Soliverez]

> On 2010-06-28 02:52:00, Alvaro Soliverez wrote:
> > trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/csvimporterdlg.ui, line 31
> > <http://reviewboard.kde.org/r/4462/diff/1/?file=29806#file29806line31>
> >
> >     Use QLayouts instead of fixed positions. This does not play well with different resolutions
> 
> Allan Anderson wrote:
>     Ah, will look into them.  Those values came from QtDesigner, not from me directly.  I did actually check different resolutions.  But, if there's a better way, then, great.
>     
>     On the subject of the ui, Cristian criticised them because of issues on resizing.  I did have problems with the layouts when I tried to make better use of grid layouts, but have had a quick look and will try to improve things.
>     
>     However, on the general issue of resizing a ui, I spent a long time on it and am not sure it should be, or needs to be resized.  The top parts don't gain anything of value if stretched, and if shrunk, lose visibility.  The only area I see that would improve on being stretched is the table widget.  However, there are scroll bars, and it meant only to be used for confirmation of column contents, not for prolonged study.
>     
>     I'm more inclined to make the whole thing of fixed size.  What does the team think?  Am I missing something?

Fixed size is difficult to maintain and not adaptable to new resolutions and to additions or substractions of items.
Use QVLayout, QHLayout, or QGridLayout, do not use fixed-size. At most, use a size-hint.


> On 2010-06-28 02:52:00, Alvaro Soliverez wrote:
> > trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/convdate.h, line 37
> > <http://reviewboard.kde.org/r/4462/diff/1/?file=29800#file29800line37>
> >
> >     No public members, please. And respect the naming scheme. Properties of a class should be called m_<name>
> 
> Allan Anderson wrote:
>     Apologies if the is a duplication, but suspect I didn't 'publish' before moving on.
>     
>     Sorry, don't know of any naming scheme.  Anything else I need to be made aware of?

Go to the kmymoney website, developers' section. Look at the developers handbook. Some stuff may be outdated, like the CVS tagging and such, though


- Alvaro


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/4462/#review6303
-----------------------------------------------------------


On 2010-06-27 03:49:22, Cristian Onet wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4462/
> -----------------------------------------------------------
> 
> (Updated 2010-06-27 03:49:22)
> 
> 
> Review request for kmymoney.
> 
> 
> Summary
> -------
> 
> Allan's csvimporter plugin
> 
> 
> Diffs
> -----
> 
>   trunk/extragear/office/kmymoney/kmymoney/plugins/CMakeLists.txt 1143143 
>   trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/CMakeLists.txt PRE-CREATION 
>   trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/convdate.h PRE-CREATION 
>   trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/convdate.cpp PRE-CREATION 
>   trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/csvdatetest.h PRE-CREATION 
>   trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/csvdatetest.cpp PRE-CREATION 
>   trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/csvimporterdlg.h PRE-CREATION 
>   trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/csvimporterdlg.cpp PRE-CREATION 
>   trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/csvimporterdlg.ui PRE-CREATION 
>   trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/csvimporterplugin.h PRE-CREATION 
>   trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/csvimporterplugin.cpp PRE-CREATION 
>   trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/investmentdlg.h PRE-CREATION 
>   trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/investmentdlg.cpp PRE-CREATION 
>   trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/investmentdlg.ui PRE-CREATION 
>   trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/kmm_csvimport.desktop PRE-CREATION 
>   trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/kmm_csvimport.rc PRE-CREATION 
>   trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/redefinedlg.h PRE-CREATION 
>   trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/redefinedlg.cpp PRE-CREATION 
>   trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/redefinedlg.ui PRE-CREATION 
> 
> Diff: http://reviewboard.kde.org/r/4462/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Cristian
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kmymoney-devel/attachments/20100628/b1ffb40e/attachment.htm