Review Request: aprs: use external QextSerialPort for TTY reading
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107536/ --- Review request for kdewin, Marble and Release Team. Description --- Instead of embedding an (old) copy of the QextSerialPort library, find for an external one; only if found enable the reading from TTY, which is otherwise disabled (leaving its configuration tab disabled). The drop of the internal QextSerialPort should also fix all the portability issues, since the plugin itself does not use any OS-dependent API, and it is then reenabled unconditionally. Hence, bug 241125 should now be fixed, and bug 237931 and bug 242039 should not happen anymore. @release-team: yes, I know this would introduce a new optional dependency, but on the other hand a copy of a 3rd party library would go away. Would this be acceptable at this point? This addresses bug 241125. http://bugs.kde.org/show_bug.cgi?id=241125 Diffs - cmake/modules/FindQextSerialPort.cmake PRE-CREATION src/plugins/render/CMakeLists.txt d82293ee782e735ff1c90e6e13d330fb7cf8563c src/plugins/render/aprs/AprsPlugin.cpp f406cec2ad665977830416aa7f5df59851a5e430 src/plugins/render/aprs/AprsTTY.cpp c65ac38b24269b608c8f3ea1452b670f9422174d src/plugins/render/aprs/CMakeLists.txt fb6ef13c80568a72a5bfcf8a2e675b969238b9f6 src/plugins/render/aprs/aprsconfig.h.in d0e6b5c4ce36080dc0e59422529c55728ff04b3a src/plugins/render/aprs/posix_qextserialport.cpp 118843f02a5c62fd708b9157e59a039dff06e238 src/plugins/render/aprs/qextserialport.h 457d831cffc4ae8c43ac7db2d85a20546eb65044 src/plugins/render/aprs/qextserialport.cpp 790e5a2701ba1291a645c4fd4b09a8a1c55d7541 src/plugins/render/aprs/qextserialport_global.h 013a6dcd4ecab97425b1286139af4f0e911c38c9 src/plugins/render/aprs/win_qextserialport.cpp 5f21d7302e61b50825f79a68b352d5b9544b3fa3 Diff: http://git.reviewboard.kde.org/r/107536/diff/ Testing --- The Aprs plugin compiles fine with and without an external QextSerialPort library. Thanks, Pino Toscano ___ release-team mailing list release-team@kde.org https://mail.kde.org/mailman/listinfo/release-team
Re: Review Request: aprs: use external QextSerialPort for TTY reading
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107536/#review22832 --- Off-topic: This patch reminds me that this ext serial port usage should msot likely be gone altogether in the near future in favour of QtSerialPort we developed for Qt4 and Qt5 in open, but that is for later. - Laszlo Papp On Nov. 30, 2012, 6:29 p.m., Pino Toscano wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107536/ --- (Updated Nov. 30, 2012, 6:29 p.m.) Review request for kdewin, Marble and Release Team. Description --- Instead of embedding an (old) copy of the QextSerialPort library, find for an external one; only if found enable the reading from TTY, which is otherwise disabled (leaving its configuration tab disabled). The drop of the internal QextSerialPort should also fix all the portability issues, since the plugin itself does not use any OS-dependent API, and it is then reenabled unconditionally. Hence, bug 241125 should now be fixed, and bug 237931 and bug 242039 should not happen anymore. @release-team: yes, I know this would introduce a new optional dependency, but on the other hand a copy of a 3rd party library would go away. Would this be acceptable at this point? This addresses bug 241125. http://bugs.kde.org/show_bug.cgi?id=241125 Diffs - cmake/modules/FindQextSerialPort.cmake PRE-CREATION src/plugins/render/CMakeLists.txt d82293ee782e735ff1c90e6e13d330fb7cf8563c src/plugins/render/aprs/AprsPlugin.cpp f406cec2ad665977830416aa7f5df59851a5e430 src/plugins/render/aprs/AprsTTY.cpp c65ac38b24269b608c8f3ea1452b670f9422174d src/plugins/render/aprs/CMakeLists.txt fb6ef13c80568a72a5bfcf8a2e675b969238b9f6 src/plugins/render/aprs/aprsconfig.h.in d0e6b5c4ce36080dc0e59422529c55728ff04b3a src/plugins/render/aprs/posix_qextserialport.cpp 118843f02a5c62fd708b9157e59a039dff06e238 src/plugins/render/aprs/qextserialport.h 457d831cffc4ae8c43ac7db2d85a20546eb65044 src/plugins/render/aprs/qextserialport.cpp 790e5a2701ba1291a645c4fd4b09a8a1c55d7541 src/plugins/render/aprs/qextserialport_global.h 013a6dcd4ecab97425b1286139af4f0e911c38c9 src/plugins/render/aprs/win_qextserialport.cpp 5f21d7302e61b50825f79a68b352d5b9544b3fa3 Diff: http://git.reviewboard.kde.org/r/107536/diff/ Testing --- The Aprs plugin compiles fine with and without an external QextSerialPort library. Thanks, Pino Toscano ___ release-team mailing list release-team@kde.org https://mail.kde.org/mailman/listinfo/release-team
Re: Review Request: aprs: use external QextSerialPort for TTY reading
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107536/#review22838 --- My release team hat says that less copied code is a good thing - Albert Astals Cid On Nov. 30, 2012, 6:29 p.m., Pino Toscano wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107536/ --- (Updated Nov. 30, 2012, 6:29 p.m.) Review request for kdewin, Marble and Release Team. Description --- Instead of embedding an (old) copy of the QextSerialPort library, find for an external one; only if found enable the reading from TTY, which is otherwise disabled (leaving its configuration tab disabled). The drop of the internal QextSerialPort should also fix all the portability issues, since the plugin itself does not use any OS-dependent API, and it is then reenabled unconditionally. Hence, bug 241125 should now be fixed, and bug 237931 and bug 242039 should not happen anymore. @release-team: yes, I know this would introduce a new optional dependency, but on the other hand a copy of a 3rd party library would go away. Would this be acceptable at this point? This addresses bug 241125. http://bugs.kde.org/show_bug.cgi?id=241125 Diffs - cmake/modules/FindQextSerialPort.cmake PRE-CREATION src/plugins/render/CMakeLists.txt d82293ee782e735ff1c90e6e13d330fb7cf8563c src/plugins/render/aprs/AprsPlugin.cpp f406cec2ad665977830416aa7f5df59851a5e430 src/plugins/render/aprs/AprsTTY.cpp c65ac38b24269b608c8f3ea1452b670f9422174d src/plugins/render/aprs/CMakeLists.txt fb6ef13c80568a72a5bfcf8a2e675b969238b9f6 src/plugins/render/aprs/aprsconfig.h.in d0e6b5c4ce36080dc0e59422529c55728ff04b3a src/plugins/render/aprs/posix_qextserialport.cpp 118843f02a5c62fd708b9157e59a039dff06e238 src/plugins/render/aprs/qextserialport.h 457d831cffc4ae8c43ac7db2d85a20546eb65044 src/plugins/render/aprs/qextserialport.cpp 790e5a2701ba1291a645c4fd4b09a8a1c55d7541 src/plugins/render/aprs/qextserialport_global.h 013a6dcd4ecab97425b1286139af4f0e911c38c9 src/plugins/render/aprs/win_qextserialport.cpp 5f21d7302e61b50825f79a68b352d5b9544b3fa3 Diff: http://git.reviewboard.kde.org/r/107536/diff/ Testing --- The Aprs plugin compiles fine with and without an external QextSerialPort library. Thanks, Pino Toscano ___ release-team mailing list release-team@kde.org https://mail.kde.org/mailman/listinfo/release-team
Re: Review Request: aprs: use external QextSerialPort for TTY reading
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107536/ --- (Updated Nov. 30, 2012, 8:24 p.m.) Review request for kdewin, Marble, Release Team, and Wes Hardaker. Changes --- Looks good to me, but I can't test it really lacking suitable hardware. I added Wes (plugin's author) as a reviewer explicitly to get him notified about it. Description --- Instead of embedding an (old) copy of the QextSerialPort library, find for an external one; only if found enable the reading from TTY, which is otherwise disabled (leaving its configuration tab disabled). The drop of the internal QextSerialPort should also fix all the portability issues, since the plugin itself does not use any OS-dependent API, and it is then reenabled unconditionally. Hence, bug 241125 should now be fixed, and bug 237931 and bug 242039 should not happen anymore. @release-team: yes, I know this would introduce a new optional dependency, but on the other hand a copy of a 3rd party library would go away. Would this be acceptable at this point? This addresses bug 241125. http://bugs.kde.org/show_bug.cgi?id=241125 Diffs - cmake/modules/FindQextSerialPort.cmake PRE-CREATION src/plugins/render/CMakeLists.txt d82293ee782e735ff1c90e6e13d330fb7cf8563c src/plugins/render/aprs/AprsPlugin.cpp f406cec2ad665977830416aa7f5df59851a5e430 src/plugins/render/aprs/AprsTTY.cpp c65ac38b24269b608c8f3ea1452b670f9422174d src/plugins/render/aprs/CMakeLists.txt fb6ef13c80568a72a5bfcf8a2e675b969238b9f6 src/plugins/render/aprs/aprsconfig.h.in d0e6b5c4ce36080dc0e59422529c55728ff04b3a src/plugins/render/aprs/posix_qextserialport.cpp 118843f02a5c62fd708b9157e59a039dff06e238 src/plugins/render/aprs/qextserialport.h 457d831cffc4ae8c43ac7db2d85a20546eb65044 src/plugins/render/aprs/qextserialport.cpp 790e5a2701ba1291a645c4fd4b09a8a1c55d7541 src/plugins/render/aprs/qextserialport_global.h 013a6dcd4ecab97425b1286139af4f0e911c38c9 src/plugins/render/aprs/win_qextserialport.cpp 5f21d7302e61b50825f79a68b352d5b9544b3fa3 Diff: http://git.reviewboard.kde.org/r/107536/diff/ Testing --- The Aprs plugin compiles fine with and without an external QextSerialPort library. Thanks, Pino Toscano ___ release-team mailing list release-team@kde.org https://mail.kde.org/mailman/listinfo/release-team
