D16279: Show progress when sending a file from desktop

[Nicolas Fella]

nicolasfella requested changes to this revision.
nicolasfella added a comment.
This revision now requires changes to proceed.


  Great work!
  
  I have a few cosmetic suggestions: Instead of saying "Sending file over KDE Connect" I'd say "Sending file to <devicename>". Then the second line should be the filename. The Jobtracker seems to pick that automatically from the info. If you remove the "To" line from the info the filename is shown as secon line. Don't ask me how exactly this works. The end result is:
  F6335741: Screenshot_20181017_211737.png <https://phabricator.kde.org/F6335741>
  
  A few code nitpicks: 
  You should remove the commented code and debug output. 
  Please follow the coding style from the rest of the file. Opening { for methods go on the next line, for everything inside methods on the same line. I'll leave a few more comments as inline comments

INLINE COMMENTS

> uploadjob.cpp:63
> +                { i18nc("File transfer origin", "From"), m_input.staticCast<QFile>().data()->fileName() },
> +                { i18nc("File transfer destination", "To"), m_deviceName }
> +    );

Use Daemon::instance()->getDevice(this->m_deviceId)->name() directly and remove m_deviceName

> uploadjob.cpp:103
> +    if (!m_timer.isValid()) {
> +            m_timer.start();
> +    }

Looks like too much indentation to me

REPOSITORY
  R224 KDE Connect

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

To: eduisters, #kde_connect, nicolasfella
Cc: nicolasfella, kdeconnect, skymoore, wistak, dvalencia, rmenezes, julioc, Leptopoda, timothyc, jdvr, yannux, Danial0_0, johnq, Pitel, adeen-s, SemperPeritus, 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/20181017/009a7d8d/attachment.html>