<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://svn.reviewboard.kde.org/r/5162/">http://svn.reviewboard.kde.org/r/5162/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 8th, 2010, 6:28 p.m., <b>Cristian Onet</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Please add the following missing files from the patch:
redefinedlgdecl.ui - missing from the patch
tick.png - present in the patch but since it's a binary it must be sent to the mailing list or directly to me (since the patch does not contain the actual binary file)
nogo.png - the same as the above</pre>
</blockquote>
<p>On September 9th, 2010, 7:59 p.m., <b>Alvaro Soliverez</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Those two png files. From the file names I assume those are icon files. Can't we use standard KDE icons for that instead of adding even more icons?
We are trying to use standard icons or have Oxygen build icons and include them in the standard set whenever possible.</pre>
</blockquote>
<p>On September 9th, 2010, 9:19 p.m., <b>Allan Anderson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Cristian made the same point to me offline.:-
> Hi,
> >
> > Would you agree to using the standard KDE icons for these actions
> > instead of adding these two icons? I think it would be better that way
> > since the two icons really resemble the KDE standard ones so it would
> > be better to avoid duplication.
"
Hi Cristian
I did try them, but felt they were a bit overpowering, with having a
number of them in a small area. I also tried the flag icons, but they
were the opposite. I'll have another look, though.
"
I've since responded as follows:
"
Hi Cristian
I've had another look, and I have to say I don't like the standard
dialog-ok and dialog-cancel in this use.
By standard KDE icons, do you mean just the the oxygen? What about
hicolor? These seem to be more suited for the drop-down -
/usr/share/icons/hicolor/16x16/apps/gdu-smart-healthy.png and
gdu-smart-failing.png, although the names aren't helpful
If not, then I guess it has to be the standard dialog-ok and dialog-cancel."
-----------------------------
I'm happy to drop my own icons if the hicolor ones are acceptable.
If it has to be the oxygen ones, then so be it, but they do jump out at you in this context.
</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Can you send a screenshot to the list to understand why the standard icons are not suitable for this? My setup is too modified at the moment to add yet another patch.</pre>
<br />
<p>- Alvaro</p>
<br />
<p>On September 8th, 2010, 10:29 p.m., Allan Anderson wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://svn.reviewboard.kde.orgrb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for kmymoney.</div>
<div>By Allan Anderson.</div>
<p style="color: grey;"><i>Updated 2010-09-08 22:29:47</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This is the revised version of the kmymoney csvimporter, which was originally submitted for me by Cristian. I was unable to find a way to update that, possibly because I was not the original submitter.
Apart from addressing the original criticisms, I've added further improvements and spent quite a bit of time tightening its error checking.
As well as the needed files, I've included csvimporterrc. This is not *needed* by the importer, as it will create one. However, as the user may wish to supplement the common basic transaction types , in order to cope with his own bank file layout idiosyncrasies, it may serve as an illustration or example. Where it should reside, I don't know. I would also wish to include some basic instructions, but in what form, and where?
Apart from functioning as a plugin, it also can produce QIF files if required.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I've run Krazy2 and astyle against it, also unit test.
Operationally, I've imported CSV files of checking/savings accounts from a number of UK and other banks. Also, investment account CSV files from a UK and a US investment institution.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/redefinedlgdecl.ui <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="http://svn.reviewboard.kde.org/r/5162/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>