krita junior job

[Dmitry Kazakov]

I'd say it's "overtype" but not "overkill".
Ok, i don't insist on these defines =)

Well, i like the patch. The last word on commit is for Boudewijn.


On Mon, Dec 14, 2009 at 11:48 AM, Adam <nospam at xibo.at> wrote:

> Hi,
> 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.
> (new patch appended)
>
> adam
>
>
> On Sun, 13 Dec 2009 22:23:49 +0100, Dmitry Kazakov <dimula73 at gmail.com>
> wrote:
>
>  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
>>>
>>>
>>>
>>
>>
>
> --
> 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/1c389929/attachment.htm