D24841: Use modern way to set the C/CXX standad

2019-10-21 Thread Hannah von Reth
vonreth updated this revision to Diff 68488. vonreth added a comment. - Raise CMake requirements to 3.5 REPOSITORY R240 Extra CMake Modules CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24841?vs=68486&id=68488 BRANCH cmake_c_standard REVISION DETAIL https://phabricator.kde.

D24841: Use modern way to set the C/CXX standad

2019-10-21 Thread David Faure
dfaure added a comment. Dunno, it's harder to justify in a standalone review :) "Increase dependency, just because" ;) REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D24841 To: vonreth, dfaure, cullmann Cc: aacid, chehrlic, kde-frameworks-devel, kde-bui

D24841: Use modern way to set the C/CXX standad

2019-10-21 Thread Hannah von Reth
vonreth added a comment. I guess we should do that in a separate review? REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D24841 To: vonreth, dfaure, cullmann Cc: aacid, chehrlic, kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, bencreasy, michaelh,

D24841: Use modern way to set the C/CXX standad

2019-10-21 Thread Hannah von Reth
vonreth updated this revision to Diff 68486. vonreth added a comment. Fix REPOSITORY R240 Extra CMake Modules CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24841?vs=68478&id=68486 BRANCH cmake_c_standard REVISION DETAIL https://phabricator.kde.org/D24841 AFFECTED FILES k

D24841: Use modern way to set the C/CXX standad

2019-10-21 Thread David Faure
dfaure added a comment. Works for me. REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D24841 To: vonreth, dfaure, cullmann Cc: aacid, chehrlic, kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, bencreasy, michaelh, ngraham, bruns

D24841: Use modern way to set the C/CXX standad

2019-10-21 Thread Albert Astals Cid
aacid added a comment. If you want to increase cmake dependency, we should at least increase it to 3.5 https://repology.org/project/cmake/badges REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D24841 To: vonreth, dfaure, cullmann Cc: aacid, chehrlic,

D24841: Use modern way to set the C/CXX standad

2019-10-21 Thread David Faure
dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > KDECompilerSettings.cmake:208 > +set(CMAKE_C_STANDARD 90) > +set(CMAKE_CXX_STANDARD_REQUIRED 11) > That's not how it works. You want set(CMAKE_CXX_STANDA

D24841: Use modern way to set the C/CXX standad

2019-10-21 Thread Hannah von Reth
vonreth updated this revision to Diff 68478. vonreth added a comment. CMAKE_CXX_STANDARD_REQUIRED REPOSITORY R240 Extra CMake Modules CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24841?vs=68475&id=68478 BRANCH cmake_c_standard REVISION DETAIL https://phabricator.kde.org/D2

D24841: Use modern way to set the C/CXX standad

2019-10-21 Thread Hannah von Reth
vonreth edited the summary of this revision. REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D24841 To: vonreth, dfaure, cullmann Cc: chehrlic, kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, bencreasy, michaelh, ngraham, bruns

D24841: Use modern way to set the C/CXX standad

2019-10-21 Thread Hannah von Reth
vonreth added a comment. In D24841#551653 , @chehrlic wrote: > I would also use CMAKE_CXX_STANDARD_REQUIRED to make sure the compiler actually *can* c++11, otherwise the flag is only added when the compiler supports it. Should not make much diff

D24841: Use modern way to set the C/CXX standad

2019-10-21 Thread Christian Ehrlicher
chehrlic added a comment. I would also use CMAKE_CXX_STANDARD_REQUIRED to make sure the compiler actually *can* c++11, otherwise the flag is only added when the compiler supports it. Should not make much difference nowadays but since it's in ECM... REPOSITORY R240 Extra CMake Modules REVI

D24841: Use modern way to set the C/CXX standad

2019-10-21 Thread Hannah von Reth
vonreth updated this revision to Diff 68475. vonreth added a comment. Change message REPOSITORY R240 Extra CMake Modules CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24841?vs=68473&id=68475 BRANCH cmake_c_standard REVISION DETAIL https://phabricator.kde.org/D24841 AFFECTE

D24841: Use modern way to set the C/CXX standad

2019-10-21 Thread Hannah von Reth
vonreth added reviewers: dfaure, cullmann. REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D24841 To: vonreth, dfaure, cullmann Cc: kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, bencreasy, michaelh, ngraham, bruns

D24841: Use modern way to set the C/CXX standad

2019-10-21 Thread Hannah von Reth
vonreth created this revision. Herald added projects: Frameworks, Build System. Herald added subscribers: kde-buildsystem, kde-frameworks-devel. vonreth requested review of this revision. REVISION SUMMARY Raise C from C89 ti C90 as C89 is not supported by the CMAKE flag https://cmake.org/c

D24826: Enforce 100 chars line width

2019-10-21 Thread Christoph Cullmann
cullmann added a comment. See the other task: columnlimit 0 leads to endless long lines and is a unusable default. REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D24826 To: romangg, #frameworks, cullmann Cc: winterz, zzag, kde-frameworks-devel, kde-build

D24826: Enforce 100 chars line width

2019-10-21 Thread Allen Winter
winterz added a comment. I am a long-time advocate of columnLimits; however, in our modern world of programming I think 100 is too short. 240 may be a bit too long: in my personal coding style scripts I try to limit to 120. even 120 is hard to achieve sometimes. I feel that 240 is a

D24826: Enforce 100 chars line width

2019-10-21 Thread Vlad Zahorodnii
zzag added a comment. I suggest to set ColumnLimit to 0 by default and allow projects to override it. REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D24826 To: romangg, #frameworks, cullmann Cc: zzag, kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_

D24826: Enforce 100 chars line width

2019-10-21 Thread Christoph Cullmann
cullmann added a comment. Perhaps easiest way to see what happens: apply it to one of your things and vary the value. REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D24826 To: romangg, #frameworks, cullmann Cc: kde-frameworks-devel, kde-buildsystem, LeGa

D24826: Enforce 100 chars line width

2019-10-21 Thread Christoph Cullmann
cullmann added a comment. It is unfortunately very brutal and in-deed, it can create very long lines if you have matching bad constructs. One can play with stuff like a limit of 160 oder whatever to limit that, but as it brutally hard break and align stuff, with e.g. 100 for my tries, a

D24826: Enforce 100 chars line width

2019-10-21 Thread Roman Gilg
romangg added a comment. In D24826#551280 , @cullmann wrote: > As explained in the thread on https://phabricator.kde.org/T11214, this will make the formatting ugly as hell, as if you have long method names, stuff is broken in arbitrary bad ways.

D24826: Enforce 100 chars line width

2019-10-21 Thread Christoph Cullmann
cullmann added a comment. As explained in the thread on https://phabricator.kde.org/T11214, this will make the formatting ugly as hell, as if you have long method names, stuff is broken in arbitrary bad ways. I don't want to change that, if nobody can avoid the resulting breakage. REPOSITO

D24826: Enforce 100 chars line width

2019-10-21 Thread Roman Gilg
romangg created this revision. romangg added reviewers: Frameworks, cullmann. Herald added projects: Frameworks, Build System. Herald added subscribers: kde-buildsystem, kde-frameworks-devel. romangg requested review of this revision. REVISION SUMMARY The KDE Frameworks style recommended a 100 c