Discussion:
[KPhotoAlbum] Fix for compile error in RemoteConnection.cpp
Robert Krawitz
2018-06-14 12:10:19 UTC
Permalink
Don't know that this is the correct fix for versions of Qt prior to
5.11, so I'm not just blindly pushing it.

diff --git a/RemoteControl/RemoteConnection.cpp b/RemoteControl/RemoteConnection.cpp
index 1dacb727..48c91689 100644
--- a/RemoteControl/RemoteConnection.cpp
+++ b/RemoteControl/RemoteConnection.cpp
@@ -105,7 +105,7 @@ void RemoteConnection::dataReceived()
std::unique_ptr<RemoteCommand> command = RemoteCommand::create(static_cast<CommandType>(id));
command->decode(stream);
protocolDebug() << qPrintable(QTime::currentTime().toString(QString::fromUtf8("hh:mm:ss.zzz")))
- << ": Received " << qPrintable(id);
+ << ": Received " << qPrintable(QString(id));

emit gotCommand(*command);
}
--
Robert Krawitz <***@alum.mit.edu>

*** MIT Engineers A Proud Tradition http://mitathletics.com ***
Member of the League for Programming Freedom -- http://ProgFree.org
Project lead for Gutenprint -- http://gimp-print.sourceforge.net

"Linux doesn't dictate how I work, I dictate how Linux works."
--Eric Crampton
Johannes Zarl-Zierl
2018-06-14 21:14:09 UTC
Permalink
Post by Robert Krawitz
Don't know that this is the correct fix for versions of Qt prior to
5.11, so I'm not just blindly pushing it.
There's already a proposed patch:
https://phabricator.kde.org/D13329

I'd say that cgiboudeaux is correct in that the qPrintable is not necessary in this context.
Given the note regarding qUtf8Printable() in the docs [1], qPrintable seems like an
incorrect choice in the first place.

[1] https://doc.qt.io/qt-5/qtglobal.html#qPrintable]

Cheers,
Johannes
Robert Krawitz
2018-06-14 22:07:38 UTC
Permalink
Post by Johannes Zarl-Zierl
Post by Robert Krawitz
Don't know that this is the correct fix for versions of Qt prior to
5.11, so I'm not just blindly pushing it.
https://phabricator.kde.org/D13329
I'd say that cgiboudeaux is correct in that the qPrintable is not necessary in this context.
Given the note regarding qUtf8Printable() in the docs [1], qPrintable seems like an incorrect choice in the first place.
[1] https://doc.qt.io/qt-5/qtglobal.html#qPrintable]
lgtm
--
Robert Krawitz <***@alum.mit.edu>

*** MIT Engineers A Proud Tradition http://mitathletics.com ***
Member of the League for Programming Freedom -- http://ProgFree.org
Project lead for Gutenprint -- http://gimp-print.sourceforge.net

"Linux doesn't dictate how I work, I dictate how Linux works."
--Eric Crampton
Robert Krawitz
2018-06-15 12:51:05 UTC
Permalink
Do I need to open an issue in phabricator for the load performance
stuff?
--
Robert Krawitz <***@alum.mit.edu>

*** MIT Engineers A Proud Tradition http://mitathletics.com ***
Member of the League for Programming Freedom -- http://ProgFree.org
Project lead for Gutenprint -- http://gimp-print.sourceforge.net

"Linux doesn't dictate how I work, I dictate how Linux works."
--Eric Crampton
Johannes Zarl-Zierl
2018-06-15 19:36:17 UTC
Permalink
Hi Robert,
Post by Robert Krawitz
Do I need to open an issue in phabricator for the load performance
stuff?
Short answer: no, but you're welcome to try it out in the future.

Longer answer: We've started using phabricator for a few months now. For now, we do so to
get a feel for the platform and to familiarize ourselves with the differential workflow.
As a side effect, it makes it easier for KDE developers to contribute small fixes (and we've
already seem an increase in smaller patches).

At some point in the future, we will probably actively encourage contributions over
phabricator.

Cheers,
Johannes

Loading...