D25682: [WIP] add initial wsdiscovery support

[David Faure]

dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> discovery.cpp:31
> +Discoverer::Discoverer() = default;
> +Discoverer::~Discoverer() = default;

Lines 30-31 duplicate lines 23-28, surely this can't link without errors?

> dnssddiscoverer.cpp:29
> +
> +KIO::UDSEntry DNSSDDiscovery::toEntry()
> +{

All toEntry() methods should be const, no?

> dnssddiscoverer.cpp:48
> +    QUrl u;
> +    u.setScheme(QStringLiteral("smb://"));
> +    u.setHost(m_service->hostName());

smb, not smb://

> dnssddiscoverer.cpp:74
> +        // RemoteService::Ptr is useless here.
> +        for (const auto &it : qAsConst(m_services)) {
> +            if (*service == *it) {

"it" is a confusing name for the reader, but it's not an iterator. It's a service pointer.
Maybe call it servicePtr?

Then *servicePtr will make it clear that you're comparing services instances.

> wsdiscoverer.cpp:287
> +    QUrl u;
> +    u.setScheme(QStringLiteral("smb://"));
> +    u.setHost(m_remote);

"smb"

> dfaure wrote in wsdiscoverer.cpp:128
> Needs a const local var to avoid a detach. Yes, range-for is annoying for Qt containers.

When the local variable is const, you don't need qAsConst on top :-)

REPOSITORY
  R320 KIO Extras

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

To: sitter, dfaure, #frameworks, #dolphin
Cc: bcooksley, ngraham, caspermeijn, davidedmundson, kde-frameworks-devel, kfm-devel, pberestov, iasensio, fprice, LeGast00n, MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, spoorun, navarromorales, firef, andrebarros, bruns, emmanuelp, mikesomov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20191213/f7162551/attachment.htm>