sitter marked 26 inline comments as done. sitter added a comment.
Yep, still looking for primarily design input. Thanks for your comments, David! INLINE COMMENTS > dfaure wrote in discovery.h:40 > Why don't these `toEntry()` methods *return* a KIO::UDSEntry instead? > It's always empty on incoming anyway. > > And UDSEntry supports moving so the "return" statement won't make a copy. Good point. We used to pass it around before, but it indeed doesn't make much sense. > dfaure wrote in dnssddiscoverer.h:41 > Why `virtual`? I thought this only mattered for diamond-shaped inheritance. > > (OTOH I remember it leads to strange order of ctors being called, so I avoid > it as much as possible) Left over from something I tried, not actually necessary indeed. > dfaure wrote in .gitlab-ci.yml:2 > (is this meant to go into kde git? just wondering) No. Well, I mean, it's a copy of the upstream repo with only the actually necessary delta applied (STATIC internal build as opposed to SHARED). > dfaure wrote in CMakeLists.txt.user:3 > Now this one I'm sure, should NOT go into git. Yeah. Slipped into git add. > dfaure wrote in wsdiscoverytargetservice.cpp:38 > const > > (this will avoid detaching m_typeList) Forwarded to upstream repo. > dfaure wrote in wsdiscoverer.cpp:209 > qAsConst > > auto & ? waitForFinished is not const. I've made the container qAsConst though. > dfaure wrote in wsdiscoverer.cpp:252 > then rewrite this code to `.at(0)` ? That's the plan, I need to read up on when exactly there'd be more than one xaddr though. While we need only one I haven't read up if any xaddr will work all the time. > dfaure wrote in wsdiscoverer.cpp:257 > especially with the data races. > > (service->endpointReference() reads data written by another thread, without > mutex) promoted the comment to a warning for now so I don't forget to deal with it before landing > dfaure wrote in wsdiscoverer.cpp:280 > Obviously unfinished :-) Yep. Only there to easily see which service produced which device entry for debugging ;) 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
