Re: Review Request 122493: Use math.h round rather than C++11 std::lround.
On Feb. 9, 2015, 12:59 a.m., Mark Gaiser wrote: -1 You use C-Style casts. Oke, the frameworks coding style doesn't seem to explicitly forbid it (casts are not mentioned), but if i recall correctly we use the Qt style + some of our own which means we should obey the Qt style [1] which does mention casts and forbids the C-Sytle cast. Secondly, you seem to be trying to get this working on a compiler that isn't supported [2]. If one of those compilers have issues with std::lround (which i seriously doubt) then we should perhaps look into replacing it with qRound. However, Qt is also moving away from it's own algorithms in favor of the stl ones so we should stick to the std:: versions. Cheers [1] http://qt-project.org/wiki/Qt_Coding_Style [2] https://community.kde.org/Frameworks/Policies#Frameworks_compiler_requirements_and_C.2B.2B11 Ivan Čukić wrote: I'm torn on this one. This is a minor edit (mainly thinking about the potential qRound version) that would fix the current master to work with an old platform. Without having any downsides (apart from possible future depreciacion of qRound). But, on the other hand, something else will probably be brought in soon that will require some more essential c++11 things, and break on osx 10.7. So, one wonders whether a patch that just prolongs the inevitable has a purpose. :) Kevin Funk wrote: Not sure if it's worth discussion this issue a lot... `qRound` isn't deprecated yet, used a lot throughout frameworks, and it doesn't look like it will be deprecated (I've yet to see a discussion on the Qt ML about this). Yet `qRound` is way more pleasant to read than `static_castint(...)`. ok for using qRound for now if it fixes build on old platforms - Marco --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122493/#review75648 --- On Feb. 8, 2015, 11:12 p.m., Jeremy Whiting wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122493/ --- (Updated Feb. 8, 2015, 11:12 p.m.) Review request for KDE Frameworks and Marco Martin. Repository: kdeclarative Description --- Use math.h round rather than C++11 std::lround. Removes dependency on C++11. Diffs - src/qmlcontrols/kquickcontrolsaddons/plotter.cpp 67ce63a943234b167165b0f3986f974bba5ff0cf Diff: https://git.reviewboard.kde.org/r/122493/diff/ Testing --- kdeclarative is able to build on OS X 10.7 with the built in XCode compiler and standard library. Thanks, Jeremy Whiting ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122493: Use math.h round rather than C++11 std::lround.
On Feb. 9, 2015, 12:59 a.m., Mark Gaiser wrote: -1 You use C-Style casts. Oke, the frameworks coding style doesn't seem to explicitly forbid it (casts are not mentioned), but if i recall correctly we use the Qt style + some of our own which means we should obey the Qt style [1] which does mention casts and forbids the C-Sytle cast. Secondly, you seem to be trying to get this working on a compiler that isn't supported [2]. If one of those compilers have issues with std::lround (which i seriously doubt) then we should perhaps look into replacing it with qRound. However, Qt is also moving away from it's own algorithms in favor of the stl ones so we should stick to the std:: versions. Cheers [1] http://qt-project.org/wiki/Qt_Coding_Style [2] https://community.kde.org/Frameworks/Policies#Frameworks_compiler_requirements_and_C.2B.2B11 I'm torn on this one. This is a minor edit (mainly thinking about the potential qRound version) that would fix the current master to work with an old platform. Without having any downsides (apart from possible future depreciacion of qRound). But, on the other hand, something else will probably be brought in soon that will require some more essential c++11 things, and break on osx 10.7. So, one wonders whether a patch that just prolongs the inevitable has a purpose. :) - Ivan --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122493/#review75648 --- On Feb. 8, 2015, 11:12 p.m., Jeremy Whiting wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122493/ --- (Updated Feb. 8, 2015, 11:12 p.m.) Review request for KDE Frameworks and Marco Martin. Repository: kdeclarative Description --- Use math.h round rather than C++11 std::lround. Removes dependency on C++11. Diffs - src/qmlcontrols/kquickcontrolsaddons/plotter.cpp 67ce63a943234b167165b0f3986f974bba5ff0cf Diff: https://git.reviewboard.kde.org/r/122493/diff/ Testing --- kdeclarative is able to build on OS X 10.7 with the built in XCode compiler and standard library. Thanks, Jeremy Whiting ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122493: Use math.h round rather than C++11 std::lround.
On Feb. 9, 2015, 12:59 a.m., Mark Gaiser wrote: -1 You use C-Style casts. Oke, the frameworks coding style doesn't seem to explicitly forbid it (casts are not mentioned), but if i recall correctly we use the Qt style + some of our own which means we should obey the Qt style [1] which does mention casts and forbids the C-Sytle cast. Secondly, you seem to be trying to get this working on a compiler that isn't supported [2]. If one of those compilers have issues with std::lround (which i seriously doubt) then we should perhaps look into replacing it with qRound. However, Qt is also moving away from it's own algorithms in favor of the stl ones so we should stick to the std:: versions. Cheers [1] http://qt-project.org/wiki/Qt_Coding_Style [2] https://community.kde.org/Frameworks/Policies#Frameworks_compiler_requirements_and_C.2B.2B11 Ivan Čukić wrote: I'm torn on this one. This is a minor edit (mainly thinking about the potential qRound version) that would fix the current master to work with an old platform. Without having any downsides (apart from possible future depreciacion of qRound). But, on the other hand, something else will probably be brought in soon that will require some more essential c++11 things, and break on osx 10.7. So, one wonders whether a patch that just prolongs the inevitable has a purpose. :) Not sure if it's worth discussion this issue a lot... `qRound` isn't deprecated yet, used a lot throughout frameworks, and it doesn't look like it will be deprecated (I've yet to see a discussion on the Qt ML about this). Yet `qRound` is way more pleasant to read than `static_castint(...)`. - Kevin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122493/#review75648 --- On Feb. 8, 2015, 11:12 p.m., Jeremy Whiting wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122493/ --- (Updated Feb. 8, 2015, 11:12 p.m.) Review request for KDE Frameworks and Marco Martin. Repository: kdeclarative Description --- Use math.h round rather than C++11 std::lround. Removes dependency on C++11. Diffs - src/qmlcontrols/kquickcontrolsaddons/plotter.cpp 67ce63a943234b167165b0f3986f974bba5ff0cf Diff: https://git.reviewboard.kde.org/r/122493/diff/ Testing --- kdeclarative is able to build on OS X 10.7 with the built in XCode compiler and standard library. Thanks, Jeremy Whiting ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 122493: Use math.h round rather than C++11 std::lround.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122493/ --- Review request for KDE Frameworks and Marco Martin. Repository: kdeclarative Description --- Use math.h round rather than C++11 std::lround. Removes dependency on C++11. Diffs - src/qmlcontrols/kquickcontrolsaddons/plotter.cpp 67ce63a943234b167165b0f3986f974bba5ff0cf Diff: https://git.reviewboard.kde.org/r/122493/diff/ Testing --- kdeclarative is able to build on OS X 10.7 with the built in XCode compiler and standard library. Thanks, Jeremy Whiting ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122493: Use math.h round rather than C++11 std::lround.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122493/#review75648 --- -1 You use C-Style casts. Oke, the frameworks coding style doesn't seem to explicitly forbid it (casts are not mentioned), but if i recall correctly we use the Qt style + some of our own which means we should obey the Qt style [1] which does mention casts and forbids the C-Sytle cast. Secondly, you seem to be trying to get this working on a compiler that isn't supported [2]. If one of those compilers have issues with std::lround (which i seriously doubt) then we should perhaps look into replacing it with qRound. However, Qt is also moving away from it's own algorithms in favor of the stl ones so we should stick to the std:: versions. Cheers [1] http://qt-project.org/wiki/Qt_Coding_Style [2] https://community.kde.org/Frameworks/Policies#Frameworks_compiler_requirements_and_C.2B.2B11 - Mark Gaiser On Feb. 8, 2015, 11:12 p.m., Jeremy Whiting wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122493/ --- (Updated Feb. 8, 2015, 11:12 p.m.) Review request for KDE Frameworks and Marco Martin. Repository: kdeclarative Description --- Use math.h round rather than C++11 std::lround. Removes dependency on C++11. Diffs - src/qmlcontrols/kquickcontrolsaddons/plotter.cpp 67ce63a943234b167165b0f3986f974bba5ff0cf Diff: https://git.reviewboard.kde.org/r/122493/diff/ Testing --- kdeclarative is able to build on OS X 10.7 with the built in XCode compiler and standard library. Thanks, Jeremy Whiting ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122493: Use math.h round rather than C++11 std::lround.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122493/#review75646 --- `qRound`? - Kevin Funk On Feb. 8, 2015, 11:12 p.m., Jeremy Whiting wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122493/ --- (Updated Feb. 8, 2015, 11:12 p.m.) Review request for KDE Frameworks and Marco Martin. Repository: kdeclarative Description --- Use math.h round rather than C++11 std::lround. Removes dependency on C++11. Diffs - src/qmlcontrols/kquickcontrolsaddons/plotter.cpp 67ce63a943234b167165b0f3986f974bba5ff0cf Diff: https://git.reviewboard.kde.org/r/122493/diff/ Testing --- kdeclarative is able to build on OS X 10.7 with the built in XCode compiler and standard library. Thanks, Jeremy Whiting ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
