Review Request 109524: Remove some compile warnings

Stephen Kelly steveire at gmail.com
Mon Mar 18 14:36:13 UTC 2013



> On March 16, 2013, 7:22 p.m., Stephen Kelly wrote:
> > tier1/kcodecs/autotests/kcharsetstest.cpp, line 135
> > <http://git.reviewboard.kde.org/r/109524/diff/1/?file=120105#file120105line135>
> >
> >     Warnings are sometimes valid and create a todo list. Changes like this look like just hiding the warning for the sake of it. It looks like 'done' for this is something different, not just removing the variable.
> >     
> >     Please double check that the other warnings you are resolving should really be resolved in a naive way.
> 
> Miquel Canes Gonzalez wrote:
>     I think that this variable is a copy&paste from the other tests of this class. In this test we also have a FIX comment. Why don't remove the warning?
>

Because in 'fixing the warning', you also remove the compiler-generated TODO list item to implement the correct fix.

Does changing this line make anything better, apart from making the compiler not warn? What is the value of invoking  KCharsets::charsets(); in the test and not doing anything else?


- Stephen


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


On March 16, 2013, 6:16 p.m., Miquel Canes Gonzalez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109524/
> -----------------------------------------------------------
> 
> (Updated March 16, 2013, 6:16 p.m.)
> 
> 
> Review request for KDE Frameworks and Aleix Pol Gonzalez.
> 
> 
> Description
> -------
> 
> Remove some compile warnings like unused variables, reordering initializers, uint -> int...
> 
> 
> Diffs
> -----
> 
>   interfaces/kimproxy/library/kimproxy.cpp 66e263f 
>   kdecore/io/kdebug.cpp 6fd7584 
>   kdeui/dialogs/kaboutapplicationpersonmodel_p.cpp 717a6cd 
>   kdeui/dialogs/kedittoolbar.cpp 401a292 
>   kdeui/dialogs/kinputdialog.cpp 28ce074 
>   kdeui/dialogs/kshortcutseditordelegate.cpp d458106 
>   kdeui/kernel/kstyle.cpp 681b940 
>   kdeui/tests/kreplacetest.h 72674eb 
>   kdeui/tests/kreplacetest.cpp c59bd76 
>   kdeui/util/kmodifierkeyinfoprovider_dummy.cpp d456e82 
>   kdeui/widgets/kdatetable.cpp 4d24290 
>   kdeui/widgets/kkeysequencewidget.cpp 28eb47e 
>   kio/bookmarks/kbookmarkimporter_opera.cc d39f7a4 
>   kio/kio/ksambashare.cpp 0239af5 
>   kio/tests/kbookmarktest.cpp 3a0c742 
>   kioslave/file/file.cpp d57517b 
>   knewstuff/knewstuff3/ui/itemsgridviewdelegate.cpp eb72240 
>   staging/ki18n/src/klocalizedstring.cpp 452dd59 
>   staging/kwidgets/src/icons/kiconengine.cpp 3e5fcc0 
>   tier1/itemmodels/src/kdescendantsproxymodel.cpp 8ee0702 
>   tier1/itemmodels/src/kselectionproxymodel.cpp f002f44 
>   tier1/kcodecs/autotests/kcharsetstest.cpp 0c7aac1 
>   tier1/kjs/src/kjs/array_object.cpp b726c09 
>   tier1/kjs/src/kjs/bytecode/machine.cpp.in a434c36 
>   tier1/kwindowsystem/src/kxutils.cpp f3e276a 
>   tier1/solid/src/solid/backends/upnp/upnpinternetgateway.cpp 2c4ca85 
> 
> Diff: http://git.reviewboard.kde.org/r/109524/diff/
> 
> 
> Testing
> -------
> 
> compile again with less warnings.
> 
> 
> Thanks,
> 
> Miquel Canes Gonzalez
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20130318/d35eaa5f/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list