D18167: Move -Wsuggest-override -Wlogical-op to regular compiler settings

kossebau added a comment. In D18167#398360 , @aacid wrote: > In D18167#398343 , @kossebau wrote: > > > only 3(?) days between proposal and commit was also a very rushy > > > Check your dates

D18167: Move -Wsuggest-override -Wlogical-op to regular compiler settings

aacid added a comment. In D18167#398076 , @graesslin wrote: > The human error exists as long as clang-tidy is not used. What I fear is that someone does a hand porting - we have seen several attempts to do that in KWin from various developers.

D18167: Move -Wsuggest-override -Wlogical-op to regular compiler settings

aacid added a comment. In D18167#398343 , @kossebau wrote: > only 3(?) days between proposal and commit was also a very rushy Check your dates better please, it's 9 days > Though then in this very case, my own take is to be pragmatic

D18167: Move -Wsuggest-override -Wlogical-op to regular compiler settings

kossebau added a comment. In D18167#398076 , @graesslin wrote: > The human error exists as long as clang-tidy is not used. What I fear is that someone does a hand porting - we have seen several attempts to do that in KWin from various

D18167: Move -Wsuggest-override -Wlogical-op to regular compiler settings

zzag added a comment. In D18167#398076 , @graesslin wrote: > The human error exists as long as clang-tidy is not used. What I fear is that someone does a hand porting - we have seen several attempts to do that in KWin from various developers.

D18167: Move -Wsuggest-override -Wlogical-op to regular compiler settings

davidedmundson added a comment. Almost every project has already been gone over with clang-tidy. Including kwin which was then force-pushed back by you. This was back in June 2017. I've got little sympathy if we have a warning after explicitly reverting the fix to the warning. I

D18167: Move -Wsuggest-override -Wlogical-op to regular compiler settings

graesslin added a comment. The human error exists as long as clang-tidy is not used. What I fear is that someone does a hand porting - we have seen several attempts to do that in KWin from various developers. If devs don't know and now fix the warnings, they can bring in human error.

D18167: Move -Wsuggest-override -Wlogical-op to regular compiler settings

kossebau added a comment. In D18167#397319 , @graesslin wrote: > For done code this warning is pointless and negative. I invite you to work with a code base like KWin where it is more important to have a working git blame than protection for

D18167: Move -Wsuggest-override -Wlogical-op to regular compiler settings

graesslin added a comment. For done code this warning is pointless and negative. I invite you to work with a code base like KWin where it is more important to have a working git blame than protection for theoretical problems. Nobody will be able to guarantee that a 500+ change to add

D18167: Move -Wsuggest-override -Wlogical-op to regular compiler settings

aacid added a comment. In D18167#397020 , @graesslin wrote: > This causes in KWin 500+ new warnings. Do you really think it's a good idea to spam all of KDE with new compiler warnings. KDE has an old code base. We cannot enable warnings for

D18167: Move -Wsuggest-override -Wlogical-op to regular compiler settings

graesslin added a comment. This causes in KWin 500+ new warnings. Do you really think it's a good idea to spam all of KDE with new compiler warnings. KDE has an old code base. We cannot enable warnings for the way you developed C++ for 20 years. REPOSITORY R240 Extra CMake Modules

D18167: Move -Wsuggest-override -Wlogical-op to regular compiler settings

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit R240:7d73c6744f64: Move -Wsuggest-override -Wlogical-op to regular compiler settings (authored by aacid). REPOSITORY

D18167: Move -Wsuggest-override -Wlogical-op to regular compiler settings

aacid added a comment. Since i have two +1 i'll commit this next saturday unless someone shouts in disagreement REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D18167 To: aacid Cc: apol, vkrause, kde-frameworks-devel, kde-buildsystem, michaelh, ngraham,

D18167: Move -Wsuggest-override -Wlogical-op to regular compiler settings

apol added a comment. +1 to me too. REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D18167 To: aacid Cc: apol, vkrause, kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns

D18167: Move -Wsuggest-override -Wlogical-op to regular compiler settings

vkrause added a comment. IMHO a good idea, +1. REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D18167 To: aacid Cc: vkrause, kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns

D18167: Move -Wsuggest-override -Wlogical-op to regular compiler settings

aacid created this revision. Herald added projects: Frameworks, Build System. Herald added subscribers: kde-buildsystem, kde-frameworks-devel. aacid requested review of this revision. REVISION SUMMARY They really help making the code better so it's good to have all applications getting those