Re: [Development] Cleaner code base patches
On terça-feira, 22 de janeiro de 2013 23.26.07, Thiago Macieira wrote: 1) Qt library modules are compiled with -Werror (optional, via whitelist) 2) Direct compilation of all headers (mandatory) Both features are enabled only if -developer-build is passed. I'm going to assume implied consent for the features. We just need further opinions on where to enable the -Werror optional testing. Options are: 1) always with -developer-build 2) on by default on -developer-build, but the CI disables it 3) completely optional, CI enables it 4) completely optional, not enabled by the CI I favour options 1 or 2. -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center signature.asc Description: This is a digitally signed message part. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Cleaner code base patches
On 28 January 2013 09:57, Thiago Macieira thiago.macie...@intel.com wrote: Options are: 1) always with -developer-build 2) on by default on -developer-build, but the CI disables it 3) completely optional, CI enables it 4) completely optional, not enabled by the CI I favour options 1 or 2. I would vote for 1), but it can cause problems in bigger releases (remember amount of warnings before Qt5 alpha?). ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Cleaner code base patches
On segunda-feira, 28 de janeiro de 2013 16.06.25, Knoll Lars wrote: On Jan 28, 2013, at 10:01 AM, Tomasz Siekierda sierd...@gmail.com wrote: On 28 January 2013 09:57, Thiago Macieira thiago.macie...@intel.com wrote: Options are: 1) always with -developer-build 2) on by default on -developer-build, but the CI disables it 3) completely optional, CI enables it 4) completely optional, not enabled by the CI I favour options 1 or 2. I would vote for 1), but it can cause problems in bigger releases (remember amount of warnings before Qt5 alpha?). I'd favor (2) for now, and move over to (1) once we gained a little experience with it for the compilation of the whole module. The header compilation test should probably move to (1) immediately. Ok, thanks. I'll modify the patch to introduce a configure-time option. -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center signature.asc Description: This is a digitally signed message part. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Cleaner code base patches
On 2013-01-28, Knoll Lars lars.kn...@digia.com wrote: 1) always with -developer-build 2) on by default on -developer-build, but the CI disables it 3) completely optional, CI enables it 4) completely optional, not enabled by the CI I'd favor (2) for now, and move over to (1) once we gained a little experience with it for the compilation of the whole module. The header compilation test should probably move to (1) immediately. Can something in the 'middle' be done where CI somehow yells at people who introduces new warnings? /Sune ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Cleaner code base patches
On 28 Jan 2013, at 5:39 PM, Sune Vuorela wrote: On 2013-01-28, Knoll Lars lars.kn...@digia.com wrote: 1) always with -developer-build 2) on by default on -developer-build, but the CI disables it 3) completely optional, CI enables it 4) completely optional, not enabled by the CI I'd favor (2) for now, and move over to (1) once we gained a little experience with it for the compilation of the whole module. The header compilation test should probably move to (1) immediately. Can something in the 'middle' be done where CI somehow yells at people who introduces new warnings? That could maybe be another bot like the doc and sanity bots. Or maybe the sanity bot could do it. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Cleaner code base patches
On segunda-feira, 28 de janeiro de 2013 17.03.39, Rutledge Shawn wrote: On 28 Jan 2013, at 5:39 PM, Sune Vuorela wrote: On 2013-01-28, Knoll Lars lars.kn...@digia.com wrote: 1) always with -developer-build 2) on by default on -developer-build, but the CI disables it 3) completely optional, CI enables it 4) completely optional, not enabled by the CI I'd favor (2) for now, and move over to (1) once we gained a little experience with it for the compilation of the whole module. The header compilation test should probably move to (1) immediately. Can something in the 'middle' be done where CI somehow yells at people who introduces new warnings? That could maybe be another bot like the doc and sanity bots. Or maybe the sanity bot could do it. I'd rather skip that step and simply go to -Werror. Lars proposed that we experiment with it a little and see how far it goes before turning it on. The good thing about -Werror is that the failure comes fast. -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center signature.asc Description: This is a digitally signed message part. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Cleaner code base patches
On 2013-01-28, Thiago Macieira thiago.macie...@intel.com wrote: I'd rather skip that step and simply go to -Werror. Lars proposed that we experiment with it a little and see how far it goes before turning it on. The good thing about -Werror is that the failure comes fast. The bad thing about -Werror is that it triggers different issues with different optimization levels, different compilers and different compiler versions. /Sune ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Cleaner code base patches
On segunda-feira, 28 de janeiro de 2013 18.22.26, Sune Vuorela wrote: On 2013-01-28, Thiago Macieira thiago.macie...@intel.com wrote: I'd rather skip that step and simply go to -Werror. Lars proposed that we experiment with it a little and see how far it goes before turning it on. The good thing about -Werror is that the failure comes fast. The bad thing about -Werror is that it triggers different issues with different optimization levels, different compilers and different compiler versions. Understood, which is why I am not proposing to turn it on for developer builds and nothing but. That means regular users would not be affected by them and we should catch most of the issues before integration. It also means the first person to upgrade to a new compiler (hello Olivier!) needs to fix the new warnings. -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center signature.asc Description: This is a digitally signed message part. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
[Development] Cleaner code base patches
I've just pushed a set of patches for review that implement a more direct way of ensuring our codebase is cleaner (patches 45529 to 45533). You may have noticed, if I added you to one of my reviews in the past months, that I sent lots of fixes for warnings. If you paid attention to them, you may have noticed that they were all compiled with -Werror. Here's what my patches implement: 1) Qt library modules are compiled with -Werror (optional, via whitelist) I've implemented support for -Werror for both GCC and for Clang. I've also cleaned up all the warnings that I could find and were fixable for GCC 4.7 and Apple Clang (current). The idea is that we'll keep GCC 4.5 and up, Clang 3.1 and up, Apple Clang 4.0 and up clean of warnings. Modules declare that they are clean of warnings (and should be kept clean) by: CONFIG += qt_warnings_are_errors Possibly qualified by a compiler: *-g++*: CONFIG += qt_warnings_are_errors Additionally, it's possible to add some warning-disabling targets to the WERROR variable, for example: *-g++*|*-clang*: WERROR += -Wno-error=strict-aliasing Current status is that all qtbase libraries, qtsvg, qtxmlpatterns, qtquick1, qttools and qtimageformats compile with -Werror. qtscript will never compile with -Werror, qtwebkit manages this setting on its own, qtdeclarative currently has major strict-aliasing issues. 2) Direct compilation of all headers (mandatory) syncqt will generate a list of all public headers (not including the forwarding headers) and then we'll compile each header, directly, with -Werror and a few other settings, like QT_NO_KEYWORDS, QT_NO_CAST_FROM_ASCII, - Woverloaded-virtual, etc. This is a replacement for the existing tst_headers. Instead of compiling the headers all together, this will compile each header, one by one, and will catch mistakes like a header missing dependencies and or not compiling without precompiled headers. This is implemented now for GCC, Clang and ICC. This increases the build time by about 20%. I've also tested all modules and the only missing fix is the binary-compatibility one for qtmultimedia. Both features are enabled only if -developer-build is passed. -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center signature.asc Description: This is a digitally signed message part. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
