Re: Review Request 127559: BUG 360129 Do not fetch from csvimporterrc if it's empty

2016-04-08 Thread Christian David 


> On April 7, 2016, 9:59 p.m., Christian David wrote:
> > kmymoney/plugins/csvimport/investprocessing.cpp, line 1967
> > 
> >
> > Should become
> > ```m_shrsinList = profilesGroup.readEntry("ShrsinParam", 
> > m_shrsinList);```
> > 
> > The if() is very long and not needed here. However, I still do not know 
> > if this is the issue. Also the ```i18nc()s``` from ```init()``` could go 
> > here if the readSettings method is always called, which I do not know 
> > either.
> 
> Allan Anderson wrote:
> > Should become
> > m_shrsinList = profilesGroup.readEntry("ShrsinParam", m_shrsinList);
> 
> I'm not sure I understand this.  The second parameter is the default 
> value to return if the key is not found.  What does it achieve in this case?
> 
> > The if() is very long and not needed here.
> 
> There are several ifs around here, but I don't see an unduly long one.
> 
> > Also the i18nc()s from init() 
> > could go here if the readSettings method is always called, which I do 
> not know either.
> 
> readSettings is called only once, from void 
> InvestProcessing::slotFileDialogClicked(), so that code could be moved 
> somewhere in void InvestProcessing::readSettings(), I think.
> 
> Łukasz Wojniłowicz wrote:
> > Should become 
> > m_shrsinList = profilesGroup.readEntry("ShrsinParam", m_shrsinList);
> 
> I compiled KMyMoney code according to your change for every m_XXXList 
> variable and ran my test case. Proposed line looks neat but doesn't work for 
> me. SellParam= etc. are empty in my csvimporterrc after just created new 
> profile.
>  
> >readSettings is called only once, from void 
> InvestProcessing::slotFileDialogClicked(), so that code could be moved 
> somewhere in void InvestProcessing::readSettings(), I think.
> 
> Please give a code and I'll test it.
> 
> > I do not know the full conversation but I am pretty sure this patch 
> will not solve the issue. If something in the newly created rc file is 
> missing, the write method seems to fail, not the read method.
> 
> My loose observation: Write method is called at the end of importing and 
> read method is called after creating new importing profile for investment.

> I do not know the full conversation but I am pretty sure this patch will not 
> solve the issue. If something in the newly created rc file is missing, the 
> write method seems to fail, not the read method.

I withdrew this idea. You should not waste your time with it.

> I compiled KMyMoney code according to your change for every m_XXXList 
> variable and ran my test case. Proposed line looks neat but doesn't work for 
> me. SellParam= etc. are empty in my csvimporterrc after just created new 
> profile.

You are right. The code recomended by me has a different behaviour. However, 
now I doubt that anything I wrote was actually helpfull. I just briefly 
inspected the code – now I see that is more complex than I thougt. So my 
recomendations are based on insufficent knowledge. Due to the description in 
the bug report I still think there is a high chance that the issue is in the 
write function.


- Christian


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127559/#review94397
---


On April 3, 2016, 6:45 a.m., Łukasz Wojniłowicz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127559/
> ---
> 
> (Updated April 3, 2016, 6:45 a.m.)
> 
> 
> Review request for KMymoney.
> 
> 
> Bugs: 360129
> http://bugs.kde.org/show_bug.cgi?id=360129
> 
> 
> Repository: kmymoney
> 
> 
> Description
> ---
> 
> Fixes bug #360129. During creation of new investment statement
> template, transaction types are initialized in
> investprocessing.cpp, but then are overridden with empty fields
> from profile that was just created in csvimporterrc which results
> in every non-buy transaction unrecognized during the import.
> 
> 
> Diffs
> -
> 
>   kmymoney/plugins/csvimport/investprocessing.cpp 3879819 
> 
> Diff: https://git.reviewboard.kde.org/r/127559/diff/
> 
> 
> Testing
> ---
> 
> Tested using financial statement from bug #360129.
> 
> 
> Thanks,
> 
> Łukasz Wojniłowicz
> 
>

Re: Review Request 127559: BUG 360129 Do not fetch from csvimporterrc if it's empty

2016-04-08 Thread Łukasz Wojniłowicz 


> On April 7, 2016, 7:59 p.m., Christian David wrote:
> > kmymoney/plugins/csvimport/investprocessing.cpp, line 1967
> > 
> >
> > Should become
> > ```m_shrsinList = profilesGroup.readEntry("ShrsinParam", 
> > m_shrsinList);```
> > 
> > The if() is very long and not needed here. However, I still do not know 
> > if this is the issue. Also the ```i18nc()s``` from ```init()``` could go 
> > here if the readSettings method is always called, which I do not know 
> > either.
> 
> Allan Anderson wrote:
> > Should become
> > m_shrsinList = profilesGroup.readEntry("ShrsinParam", m_shrsinList);
> 
> I'm not sure I understand this.  The second parameter is the default 
> value to return if the key is not found.  What does it achieve in this case?
> 
> > The if() is very long and not needed here.
> 
> There are several ifs around here, but I don't see an unduly long one.
> 
> > Also the i18nc()s from init() 
> > could go here if the readSettings method is always called, which I do 
> not know either.
> 
> readSettings is called only once, from void 
> InvestProcessing::slotFileDialogClicked(), so that code could be moved 
> somewhere in void InvestProcessing::readSettings(), I think.

> Should become 
> m_shrsinList = profilesGroup.readEntry("ShrsinParam", m_shrsinList);

I compiled KMyMoney code according to your change for every m_XXXList variable 
and ran my test case. Proposed line looks neat but doesn't work for me. 
SellParam= etc. are empty in my csvimporterrc after just created new profile.
 
>readSettings is called only once, from void 
>InvestProcessing::slotFileDialogClicked(), so that code could be moved 
>somewhere in void InvestProcessing::readSettings(), I think.

Please give a code and I'll test it.

> I do not know the full conversation but I am pretty sure this patch will not 
> solve the issue. If something in the newly created rc file is missing, the 
> write method seems to fail, not the read method.

My loose observation: Write method is called at the end of importing and read 
method is called after creating new importing profile for investment.


- Łukasz


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127559/#review94397
---


On April 3, 2016, 4:45 a.m., Łukasz Wojniłowicz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127559/
> ---
> 
> (Updated April 3, 2016, 4:45 a.m.)
> 
> 
> Review request for KMymoney.
> 
> 
> Bugs: 360129
> http://bugs.kde.org/show_bug.cgi?id=360129
> 
> 
> Repository: kmymoney
> 
> 
> Description
> ---
> 
> Fixes bug #360129. During creation of new investment statement
> template, transaction types are initialized in
> investprocessing.cpp, but then are overridden with empty fields
> from profile that was just created in csvimporterrc which results
> in every non-buy transaction unrecognized during the import.
> 
> 
> Diffs
> -
> 
>   kmymoney/plugins/csvimport/investprocessing.cpp 3879819 
> 
> Diff: https://git.reviewboard.kde.org/r/127559/diff/
> 
> 
> Testing
> ---
> 
> Tested using financial statement from bug #360129.
> 
> 
> Thanks,
> 
> Łukasz Wojniłowicz
> 
>

Re: Build fails

2016-04-08 Thread aga 

On 08/04/16 15:31, Thomas Baumgart wrote:

Allan,

did you try to do s.th. like

cd /home/aga/GIT360435/kmymoney
rm -rf build
mkdir build
cd build
cmake ..
make



Thanks Thomas.  I realised in bed that that was what was needed, and all 
was OK afterwords.  My brain is pretty fogged up recently.


Allan



On Thursday 07 April 2016 23:20:36 aga wrote:


Please, can somebody tell me how I should fix this.
"
make
[  0%] Automatic moc for target kgpgfile
Generating moc_kgpgfile.cpp
No such file or directory
AUTOGEN: error: process for
/home/aga/GIT360435/kmymoney/build/libkgpgfile/moc_kgpgfile.cpp failed:
No such file or directory
moc failed...
libkgpgfile/CMakeFiles/kgpgfile_automoc.dir/build.make:49: recipe for
target 'libkgpgfile/CMakeFiles/kgpgfile_automoc' failed
make[2]: *** [libkgpgfile/CMakeFiles/kgpgfile_automoc] Error 1
CMakeFiles/Makefile2:142: recipe for target
'libkgpgfile/CMakeFiles/kgpgfile_automoc.dir/all' failed
make[1]: *** [libkgpgfile/CMakeFiles/kgpgfile_automoc.dir/all] Error 2
Makefile:147: recipe for target 'all' failed
make: *** [all] Error 2
"
Thanks

Allan

Re: Build fails

2016-04-08 Thread Thomas Baumgart 
Allan,

did you try to do s.th. like

cd /home/aga/GIT360435/kmymoney
rm -rf build
mkdir build
cd build 
cmake ..
make



On Thursday 07 April 2016 23:20:36 aga wrote:

> Please, can somebody tell me how I should fix this.
> "
> make
> [  0%] Automatic moc for target kgpgfile
> Generating moc_kgpgfile.cpp
> No such file or directory
> AUTOGEN: error: process for
> /home/aga/GIT360435/kmymoney/build/libkgpgfile/moc_kgpgfile.cpp failed:
> No such file or directory
> moc failed...
> libkgpgfile/CMakeFiles/kgpgfile_automoc.dir/build.make:49: recipe for
> target 'libkgpgfile/CMakeFiles/kgpgfile_automoc' failed
> make[2]: *** [libkgpgfile/CMakeFiles/kgpgfile_automoc] Error 1
> CMakeFiles/Makefile2:142: recipe for target
> 'libkgpgfile/CMakeFiles/kgpgfile_automoc.dir/all' failed
> make[1]: *** [libkgpgfile/CMakeFiles/kgpgfile_automoc.dir/all] Error 2
> Makefile:147: recipe for target 'all' failed
> make: *** [all] Error 2
> "
> Thanks
> 
> Allan
-- 

Regards

Thomas Baumgart

GPG-FP: E55E D592 F45F 116B 8429   4F99 9C59 DB40 B75D D3BA
-
The words GUESS and KNOW when used in the context of
flying may produce entirely opposite results.
-


signature.asc
Description: This is a digitally signed message part.

Re: Review Request 127559: BUG 360129 Do not fetch from csvimporterrc if it's empty

2016-04-08 Thread Allan Anderson 


> On April 7, 2016, 7:59 p.m., Christian David wrote:
> > kmymoney/plugins/csvimport/investprocessing.cpp, line 1967
> > 
> >
> > Should become
> > ```m_shrsinList = profilesGroup.readEntry("ShrsinParam", 
> > m_shrsinList);```
> > 
> > The if() is very long and not needed here. However, I still do not know 
> > if this is the issue. Also the ```i18nc()s``` from ```init()``` could go 
> > here if the readSettings method is always called, which I do not know 
> > either.

> Should become
> m_shrsinList = profilesGroup.readEntry("ShrsinParam", m_shrsinList);

I'm not sure I understand this.  The second parameter is the default value to 
return if the key is not found.  What does it achieve in this case?

> The if() is very long and not needed here.

There are several ifs around here, but I don't see an unduly long one.

> Also the i18nc()s from init() 
> could go here if the readSettings method is always called, which I do not 
> know either.

readSettings is called only once, from void 
InvestProcessing::slotFileDialogClicked(), so that code could be moved 
somewhere in void InvestProcessing::readSettings(), I think.


- Allan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127559/#review94397
---


On April 3, 2016, 4:45 a.m., Łukasz Wojniłowicz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127559/
> ---
> 
> (Updated April 3, 2016, 4:45 a.m.)
> 
> 
> Review request for KMymoney.
> 
> 
> Bugs: 360129
> http://bugs.kde.org/show_bug.cgi?id=360129
> 
> 
> Repository: kmymoney
> 
> 
> Description
> ---
> 
> Fixes bug #360129. During creation of new investment statement
> template, transaction types are initialized in
> investprocessing.cpp, but then are overridden with empty fields
> from profile that was just created in csvimporterrc which results
> in every non-buy transaction unrecognized during the import.
> 
> 
> Diffs
> -
> 
>   kmymoney/plugins/csvimport/investprocessing.cpp 3879819 
> 
> Diff: https://git.reviewboard.kde.org/r/127559/diff/
> 
> 
> Testing
> ---
> 
> Tested using financial statement from bug #360129.
> 
> 
> Thanks,
> 
> Łukasz Wojniłowicz
> 
>