D14597: [WIP] Get rid of DownloadJob

Albert Vaca Cintora noreply at phabricator.kde.org
Sun Aug 5 17:05:34 BST 2018


albertvaka added a comment.


  Indeed, this can be removed.
  
  This patch seems mergeable as it is, maybe the fix for the test can be a separate patch?

INLINE COMMENTS

> landevicelink.cpp:124
>          QVariantMap transferInfo = packet.payloadTransferInfo();
>          //FIXME: The next two lines shouldn't be needed! Why are they here?
>          transferInfo.insert(QStringLiteral("useSsl"), true);

Have you tried if removing these lines breaks something? At least `useSsl` is never checked for sure. Can be a good time to remove it.

> landevicelink.cpp:136
> +
> +        socket->connectToHostEncrypted(m_socketLineReader->peerAddress().toString(), transferInfo[QStringLiteral("port")].toInt(), QIODevice::ReadWrite);
> +        packet.setPayload(socket, packet.payloadSize());

This would be more legible if split in three lines:

  QString address = ...;
  quint16 port = ...;
  socket->connectToHostEncrypted(address, port, QIODevice::ReadWrite);

REPOSITORY
  R224 KDE Connect

REVISION DETAIL
  https://phabricator.kde.org/D14597

To: nicolasfella, #kde_connect
Cc: albertvaka, kdeconnect, rmenezes, julioc, Leptopoda, timothyc, jdvr, yannux, Danial0_0, johnq, Pitel, adeen-s, SemperPeritus, ndavis, daniel.z.tg, jeanv, seebauer, bugzy, MayeulC, menasshock, tctara, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdeconnect/attachments/20180805/892f9360/attachment-0001.html>


More information about the KDEConnect mailing list