I'd say it's "overtype" but not "overkill".<br>Ok, i don't insist on these defines =)<br><br>Well, i like the patch. The last word on commit is for Boudewijn.<br><br><br><div class="gmail_quote">
On Mon, Dec 14, 2009 at 11:48 AM, Adam <span dir="ltr"><<a href="mailto:nospam@xibo.at">nospam@xibo.at</a>></span> wrote:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
Hi,<br>
i agree with 1) and 2), but isn't defining two macros for two conversions kind of an overkill? Imo a comment would be sufficient.<br>
(new patch appended)<br><font color="#888888">
<br>
adam</font><div><div></div><div class="h5"><br>
<br>
On Sun, 13 Dec 2009 22:23:49 +0100, Dmitry Kazakov <<a href="mailto:dimula73@gmail.com" target="_blank">dimula73@gmail.com</a>> wrote:<br>
<br>
<blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
Hi, Adam!<br>
Good work!<br>
<br>
I think some fixes could make code even better:<br>
<br>
1)<br>
<br>
- if(autosaveDelay>0) {<br>
- m_autosaveCheckBox->setChecked(true);<br>
- }<br>
- else m_autosaveCheckBox->setChecked(false);<br>
<br>
+m_autosaveCheckBox->setChecked(autosaveDelay>0);<br>
<br>
2) You name the methods and a parameter in a config "AutoSave". Maybe change<br>
it to "AutoSaveInterval"? It's more self-explanatory =)<br>
<br>
3) And the last suggestion: divisions and multiplications in the code are<br>
not very self-describing.<br>
I mean this:<br>
+ m_autosaveSpinBox->setValue(autosaveDelay/60);<br>
<br>
Maybe add to the .cpp-file:<br>
#define MIN_TO_SEC(min) ((min)*60)<br>
#define SEC_TO_MIN(min) ((min)/60)<br>
<br>
... like we do in koffice/libs/kobase/KoUnit.h<br>
<br>
<br>
<br>
On Sun, Dec 13, 2009 at 9:35 PM, Adam <<a href="mailto:nospam@xibo.at" target="_blank">nospam@xibo.at</a>> wrote:<br>
<br>
<blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
Hi,<br>
here we go. Please don't hesitate to criticise my code, I'd like to write<br>
good code and that's only possible, if somebody tells me the flaws..<br>
<br>
adam<br>
<br>
<br>
On Sun, 13 Dec 2009 14:53:15 +0100, Boudewijn Rempt <<a href="mailto:boud@valdyas.org" target="_blank">boud@valdyas.org</a>><br>
wrote:<br>
<br>
On Sunday 13 December 2009, Adam wrote:<br>
<blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<br>
<blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
hi..<br>
i've done one of the junior jobs<br>
(<a href="http://wiki.koffice.org/index.php?title=Junior_Jobs#Krita" target="_blank">http://wiki.koffice.org/index.php?title=Junior_Jobs#Krita</a>) for krita.<br>
it's working now, but i don't know how to submit the solution. here is<br>
the<br>
bugtracker entry: <a href="http://bugs.kde.org/159045" target="_blank">http://bugs.kde.org/159045</a><br>
it's the first time, i code for an opensource project, so i've no idea,<br>
how things are organized, where to post the patch, which format and how<br>
to<br>
generate it..<br>
can anybody help?<br>
<br>
adam<br>
<br>
<br>
</blockquote>
Wonderful! If you do<br>
<br>
'svn diff > 159045.patch'<br>
<br>
in the koffice source directory, you will get a file that contains just<br>
your<br>
changes. You can mail that to this mailing list. We will look at it, we<br>
might<br>
have a couple of remarks and things that might have to be changed (but<br>
we'll<br>
be really nice about it), and then we'll decide it can go in, and I will<br>
make<br>
sure it goes into trunk. And your name goes into the About Box -- eternal<br>
fame!<br>
<br>
Then, if you've fixed something else, we'll do it the same way. If the<br>
second<br>
contribution goes in smoothly as well, and you intend to go on working on<br>
Krita, I'm going to help you get a KDE svn account, and we'll be really<br>
glad<br>
with another contributor :-).<br>
<br>
It's a lot of fun if you read this mailing list, participate in<br>
discussions,<br>
join us on irc, #krita on <a href="http://irc.freenode.net" target="_blank">irc.freenode.net</a> or post in forums on<br>
<a href="http://forums.kde.org" target="_blank">forums.kde.org</a>, as well.<br>
<br>
<br>
</blockquote>
--<br>
Using Opera's revolutionary e-mail client: <a href="http://www.opera.com/mail/" target="_blank">http://www.opera.com/mail/</a><br>
<br>
_______________________________________________<br>
kimageshop mailing list<br>
<a href="mailto:kimageshop@kde.org" target="_blank">kimageshop@kde.org</a><br>
<a href="https://mail.kde.org/mailman/listinfo/kimageshop" target="_blank">https://mail.kde.org/mailman/listinfo/kimageshop</a><br>
<br>
<br>
</blockquote>
<br>
<br>
</blockquote>
<br>
<br>
-- <br>
Using Opera's revolutionary e-mail client: <a href="http://www.opera.com/mail/" target="_blank">http://www.opera.com/mail/</a></div></div><br>_______________________________________________<br>
kimageshop mailing list<br>
<a href="mailto:kimageshop@kde.org">kimageshop@kde.org</a><br>
<a href="https://mail.kde.org/mailman/listinfo/kimageshop" target="_blank">https://mail.kde.org/mailman/listinfo/kimageshop</a><br>
<br></blockquote></div><br><br clear="all"><br>-- <br>Dmitry Kazakov<br>