[prev in list] [next in list] [prev in thread] [next in thread] 

List:       koffice-devel
Subject:    Re: Review Request: Add ability to read,
From:       Thomas Zander <zander () kde ! org>
Date:       2009-10-20 20:08:58
Message-ID: 200910202208.59396.zander () kde ! org
[Download RAW message or body]

On Tuesday 20. October 2009 15.12.12 Inge Wallin wrote:
> > On 2009-10-20 09:36:18, Thomas Zander wrote:
> > > trunk/koffice/libs/odf/KoBorder.cpp, line 513
> > > <http://reviewboard.kde.org/r/1890/diff/3/?file=13030#file13030line51
> > >3>
> > >
> > >     Please don't parse each item 4 times, store it in an intermediate
> > > variable.
>
> Copied from your own code, but I fixed it here.  I suggest you do the
> same in ParagraphStyle :-)

Your patch 6 doesn't show this fix.

[]
>> >     Please place the curly brace one line up.
>
>That will be more difficult to read, but ok

This doesn't show in your latest patch.

>> On 2009-10-20 09:36:18, Thomas Zander wrote:
>> > trunk/koffice/libs/widgets/KoPageLayout.h, line 62
>> > 
<http://reviewboard.kde.org/r/1890/diff/3/?file=13031#file13031line62>
>> >
>> >     Why did you write this? How is the operator= different from the
>> > auto generated one for a struct?
>
>Good question, but without it it doesn't compile.

That doesn't make it right  :)
Better look for the real reason. [looks] Ah, you forgot a const in the copy 
constructor of KoBorder.

-- 
Thomas Zander
_______________________________________________
koffice-devel mailing list
koffice-devel@kde.org
https://mail.kde.org/mailman/listinfo/koffice-devel
[prev in list] [next in list] [prev in thread] [next in thread]