[WIP] Get rid of DownloadJob
ClosedPublic

Authored by nicolasfella on Aug 4 2018, 12:18 PM.

Details

Summary

It doesn't do much and can be inlined into LanDeviceLink.

TODO: Figure out why sendfiletest fails

Test Plan

Receiving files still works

Diff Detail

Repository
R224 KDE Connect
Branch
nodownloadjob
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 1504
Build 1522: arc lint + arc unit
nicolasfella created this revision.Aug 4 2018, 12:18 PM
Restricted Application added a project: KDE Connect. · View Herald TranscriptAug 4 2018, 12:18 PM
Restricted Application added a subscriber: kdeconnect. · View Herald Transcript
nicolasfella requested review of this revision.Aug 4 2018, 12:18 PM
albertvaka added a subscriber: albertvaka.EditedAug 5 2018, 4:05 PM

Indeed, this can be removed.

This patch seems mergeable on its own, maybe the fix for the test can be a separate patch?

core/backends/lan/landevicelink.cpp
124

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.

136

This would be more legible if split in three lines:

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

Oh, you mean that sendfiletest fails now, after this change.

I thought you mean the test that was already failing before, but I see you already removed that one.

In that case it would be nice to fix it as part of the same patch :P

  • Hack test to pass
  • Remove unneeded lines
  • Split line for readability
albertvaka accepted this revision.Aug 7 2018, 7:38 AM
albertvaka added inline comments.
plugins/sendnotifications/notificationslistener.cpp
272 ↗(On Diff #39179)

Remove qDebug

tests/sendfiletest.cpp
126

Have you figured out why this signal is not being emitted? Can you add a comment explaining why, or at least saying "HACK: breaks test if uncommented"?

This revision is now accepted and ready to land.Aug 7 2018, 7:38 AM
This revision was automatically updated to reflect the committed changes.
nicolasfella marked 2 inline comments as done.Aug 15 2018, 3:21 PM