[Kmymoney-devel] KMyMoney - CSV Importer

[Cristian Oneţ]

În data de Vineri 25 Iunie 2010 02:21:48 allan a scris:
> You guys are light-years ahead of me, and I'm totally aware of the need
> for a more expert involvement - at the moment I'm just glad I got it all
> to work.  Now that Kmm has more or less completed the transition to
> KDE4, the need for the stand-alone version to produce QIF files
> diminishes, and with some expectation of the acceptance of the plugin
> version, that need diminishes further.  So, I'm happy to keep the
> stand-alone version out of the game, at least unless someone finds a
> need for it.

Hi Allan,

I've been doing a lot of cleanup on the code but it's still not in good enough 
shape to commit it. So instead of fixing those issues myself I thought that it 
would be a good opportunity for you to get acquainted with some of the good 
practices. I've ported the tests to QTest before your second mail with the 
QTest patch (sorry).

First of all I'm attaching the plugin as I see that it should be under the 
kmymoney/plugins folder + the patch to add it to the build system. You may see 
that there is a lot of change from what you've sent me but believe me I've got 
a reason for doing each of them. I've removed the convdatelib since I wouldn't 
make a lib out of a class with a method. I've cleaned up the build system and 
most importantly the headers. My first recommendation would be regarding the 
headers and the includes in the headers:
- don't include anything that you don't need in a header
- include implementation specific stuff in the cpp file
- use forward declarations where you can to avoid including an unneeded file

Now some other recommendations:
- try to get rid of the global variables in the cpp files; they don't make much 
sense and they could be made members if you need to keep the state between 
methods
- remove commented code
- don't make class members public unless you've got a really good reason to do 
that
- replace tr() with i18n()
- I would recommend changing the way things are now regarding building and 
installing the plugin unless you really know what you are doing

Now for the things you really need to fix before this code can be committed are 
the krazy warnings (see 
http://techbase.kde.org/Development/Tutorials/Code_Checking). You need to have 
the 'krazy2' application installed and run krazy2all in the csvimported 
directory. The output will also contain suggestions on how to fix the warnings 
and some explanation on why should they be fixed.

The layout in ui files can also be improved.

If you manage to complete these tasks I think you can get your SVN account. I 
know that it is hard to do such things without having the source under source 
control but we worked hard to remove the warnings from kmymoney and I would 
mess that up by committing code with warnings. If the other developers think 
we can commit the code and start working on that I'll commit it and you can 
get your SVN account but this being a plugin I suggest that we clean up the 
code first and commit it when it's ready.

I'll keep the current contents of the csvimporter folder and compare to the 
version which will contain the fixes of the krazy warnings. Let me know if you 
need any assistance.

-- 
Regards,

Cristian Oneţ
-------------- next part --------------
A non-text attachment was scrubbed...
Name: csvimport.tar.gz
Type: application/x-compressed-tar
Size: 28460 bytes
Desc: not available
Url : http://mail.kde.org/pipermail/kmymoney-devel/attachments/20100626/d0aded9c/attachment-0001.taz 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: csvimport.patch
Type: text/x-patch
Size: 367 bytes
Desc: not available
Url : http://mail.kde.org/pipermail/kmymoney-devel/attachments/20100626/d0aded9c/attachment-0001.patch 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
Url : http://mail.kde.org/pipermail/kmymoney-devel/attachments/20100626/d0aded9c/attachment-0001.sig