krita junior job

[Dmitry Kazakov]

Hi, Adam!
Good work!

I think some fixes could make code even better:

1)

-    if(autosaveDelay>0) {
-        m_autosaveCheckBox->setChecked(true);
-    }
-    else m_autosaveCheckBox->setChecked(false);

+m_autosaveCheckBox->setChecked(autosaveDelay>0);

2) You name the methods and a parameter in a config "AutoSave". Maybe change
it to "AutoSaveInterval"? It's more self-explanatory =)

3) And the last suggestion: divisions and multiplications in the code are
not very self-describing.
I mean this:
+ m_autosaveSpinBox->setValue(autosaveDelay/60);

Maybe add to the .cpp-file:
#define MIN_TO_SEC(min) ((min)*60)
#define SEC_TO_MIN(min) ((min)/60)

... like we do in koffice/libs/kobase/KoUnit.h



On Sun, Dec 13, 2009 at 9:35 PM, Adam <nospam at xibo.at> wrote:

> Hi,
> here we go. Please don't hesitate to criticise my code, I'd like to write
> good code and that's only possible, if somebody tells me the flaws..
>
> adam
>
>
> On Sun, 13 Dec 2009 14:53:15 +0100, Boudewijn Rempt <boud at valdyas.org>
> wrote:
>
>  On Sunday 13 December 2009, Adam wrote:
>>
>>> hi..
>>> i've done one of the junior jobs
>>> (http://wiki.koffice.org/index.php?title=Junior_Jobs#Krita) for krita.
>>> it's working now, but i don't know how to submit the solution. here is
>>> the
>>> bugtracker entry: http://bugs.kde.org/159045
>>> it's the first time, i code for an opensource project, so i've no idea,
>>> how things are organized, where to post the patch, which format and how
>>> to
>>> generate it..
>>> can anybody help?
>>>
>>> adam
>>>
>>>
>> Wonderful! If you do
>>
>> 'svn diff > 159045.patch'
>>
>> in the koffice source directory, you will get a file that contains just
>> your
>> changes. You can mail that to this mailing list. We will look at it, we
>> might
>> have a couple of remarks and things that might have to be changed (but
>> we'll
>> be really nice about it), and then we'll decide it can go in, and I will
>> make
>> sure it goes into trunk. And your name goes into the About Box -- eternal
>> fame!
>>
>> Then, if you've fixed something else, we'll do it the same way. If the
>> second
>> contribution goes in smoothly as well, and you intend to go on working on
>> Krita, I'm going to help you get a KDE svn account, and we'll be really
>> glad
>> with another contributor :-).
>>
>> It's a lot of fun if you read this mailing list, participate in
>> discussions,
>> join us on irc, #krita on irc.freenode.net or post in forums on
>> forums.kde.org, as well.
>>
>>
> --
> Using Opera's revolutionary e-mail client: http://www.opera.com/mail/
>
> _______________________________________________
> kimageshop mailing list
> kimageshop at kde.org
> https://mail.kde.org/mailman/listinfo/kimageshop
>
>


-- 
Dmitry Kazakov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kimageshop/attachments/20091214/f7ba129a/attachment.htm