leinir marked 2 inline comments as done.
leinir added a comment.

  In D16776#356741 <https://phabricator.kde.org/D16776#356741>, @apol wrote:
  
  > Where will the `All categories are missing` error fall right now?
  
  
  That is a configuration file error (which is the case, unless there's a 
network fault, which should be reported earlier). There is a corner case where 
the server might be broken and report all categories missing, even if they 
aren't (such as them having been unexported for some reason or another), but we 
can't detect that on the client, since from the perspective of a client, that 
is just the same thing as the category not existing. So, ConfigFileError 
catches that :)

INLINE COMMENTS

> apol wrote in KNSBackend.cpp:247
> Maybe at least qDebug() it?

guessing the "it" is the error message referred to in the comment below? At any 
rate, yes, debugging it out is not a bad idea at all, i'll do that :) (i do 
notice that there's very few qCDebugs going on... this might want fixing, but 
that's a bit outside the scope of this patch, of course)

> apol wrote in KNSBackend.cpp:275
> Are you setting error after passing error? This reads weird. If you want to 
> pass an empty one use `markInvalid({})`.

really only did this because it was how it was done in the old version of the 
function... But yes, it does kind of read weird, i'll swap them around to make 
it read more sensibly

REPOSITORY
  R134 Discover Software Store

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

To: leinir, #discover_software_store, apol
Cc: plasma-devel, masilva, ragreen, ixoos, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

Reply via email to