Review Request: Minor krazy2 warning fixes

Dawit Alemayehu adawit at kde.org
Tue May 1 16:38:11 BST 2012



> On May 1, 2012, 9:55 a.m., Konstantinos Smanis wrote:
> > kioslave/http/kcookiejar/kcookiejar_include.h, line 6
> > <http://git.reviewboard.kde.org/r/104785/diff/3/?file=59775#file59775line6>
> >
> >     Does it compile without the include?

Yes that compiles fine. Even if certain includes are needed the ones that should have been included are  QList and QMetaType, but neither is needed since this header is used in a generated source file that contains the necessary includes already.


> On May 1, 2012, 9:55 a.m., Konstantinos Smanis wrote:
> > kio/kfile/kfilemetadataprovider.cpp, line 396
> > <http://git.reviewboard.kde.org/r/104785/diff/3/?file=59756#file59756line396>
> >
> >     This doesn't need normalization. And since kdelibs coding style suggests padding operators, the whitespace should remain.

It is the script that did that. However, I will revert this because it is indeed not necessary.


> On May 1, 2012, 9:55 a.m., Konstantinos Smanis wrote:
> > kio/bookmarks/kbookmarkmenu.cc, line 734
> > <http://git.reviewboard.kde.org/r/104785/diff/3/?file=59754#file59754line734>
> >
> >     You haven't normalized here.

Well well well... The Qt tool only looked for ".cpp" files and ignores those whose extension is .cc or .c++ or .cxx. I fixed that and it caught these as well.


On May 1, 2012, 9:55 a.m., Dawit Alemayehu wrote:
> > I have made a few comments, mostly about code inconsistencies. Since it's a matter of taste of the maintainer I didn't open issues, except when needed.

Though I appreciate you going through the patch and finding all of those styling issues, I think you completely neglected my last response to David's inquiry of whether I did the normalization fixes by hand or not. All the normalization related changes were done by a tool. It makes no sense to do these changes otherwise. As such, the last thing I worry or care about is the padding changes the tool might introduce to a bunch of connect statements. I got no time for such trivialities. With the exception of the code I manually added and or modified I am not going to go through all these changes and fix padding issues. Sorry.


- Dawit


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104785/#review13196
-----------------------------------------------------------


On April 30, 2012, 11:50 p.m., Dawit Alemayehu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104785/
> -----------------------------------------------------------
> 
> (Updated April 30, 2012, 11:50 p.m.)
> 
> 
> Review request for kdelibs.
> 
> 
> Description
> -------
> 
> The following patch fixes the following krazy2 warnings:
> 
>  - Use const references in Q_FOREACH statements where appropriate.
>  - Normalize yet more signal/slot connections (missing from the first go round).
>  - Use brackets instead of double-quotes for the 'config*' header files.
>  - Fix the #ifdef statements in header files to reflect the header filename.
> 
> I did this a long time ago, but never pushed upstream. As part of my spring clean up I want to push this local changes upstream. Any objections ?
> 
> 
> Diffs
> -----
> 
>   kio/bookmarks/kbookmarkdialog.cc 713ceff 
>   kio/bookmarks/kbookmarkdombuilder.cc 8e0be3c 
>   kio/bookmarks/kbookmarkimporter.cc 08210f7 
>   kio/bookmarks/kbookmarkmanager.cc d8a9cb7 
>   kio/bookmarks/kbookmarkmenu.cc deb973b 
>   kio/bookmarks/konqbookmarkmenu.cc 4fc6be0 
>   kio/kfile/kfilemetadataprovider.cpp 8caa0c2 
>   kio/kfile/kfilemetadataprovider_p.h 09d924a 
>   kio/kfile/kfilemetadatareaderprocess.cpp 5103087 
>   kio/kfile/kimagefilepreview.cpp 74ef8b7 
>   kio/kio/accessmanager.cpp e535c8a 
>   kio/kio/chmodjob.cpp 85e0c2c 
>   kio/kio/job.h aeaffa2 
>   kio/kio/job.cpp 5e18998 
>   kio/kio/jobuidelegate.cpp 85679c2 
>   kio/kio/kdesktopfileactions.cpp edf2e9c 
>   kio/kio/kfileitemactions.h 27ab4e3 
>   kio/kio/kfileitemactions.cpp c79a434 
>   kio/kio/kfilemetainfoitem.cpp 1cab458 
>   kio/kio/ksambasharedata.cpp aebcb04 
>   kio/kio/kurifilter.h 289b910 
>   kio/kio/kurifilter.cpp 0144a2c 
>   kio/kio/renamedialog.cpp 11e55a9 
>   kio/misc/kpac/proxyscout.cpp 0068ce7 
>   kio/misc/kpac/script.cpp a595301 
>   kioslave/http/kcookiejar/kcookiejar_include.h 4053a53 
>   kioslave/http/tests/httpauthenticationtest.h 35b822a 
>   nepomuk/core/resourcedata.cpp 0a5295b 
>   nepomuk/query/dateparser.cpp 9bd2e1a 
>   nepomuk/rcgen/ontologyparser.cpp f9f8673 
>   nepomuk/rcgen/property.cpp 51d9c07 
>   nepomuk/types/class.cpp 0210537 
>   nepomuk/types/ontology.cpp 124e88c 
>   nepomuk/types/ontologymanager.cpp 10d1031 
>   nepomuk/types/property.cpp 06daf3c 
>   nepomuk/utils/datefacet.cpp 386aa00 
> 
> Diff: http://git.reviewboard.kde.org/r/104785/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dawit Alemayehu
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20120501/737cf63c/attachment.htm>


More information about the kde-core-devel mailing list