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


  Thinking this is... well under way, with a couple of details :)

INLINE COMMENTS

> kauthactionreply.h:338
> -*
> -* In case of errors coming from the library, the type() is
> -* ActionReply::KAuthError. In this case, errorCode() will always be one of 
> the

Thinking it would be kind of good to translate this into something more useful 
for the Action::AuthStatus enum documentation... Just removing it entirely 
seems to me to get rid of the verbose "don't use this for your own errors, this 
is for authentication only, you should look over here" thing which is inherent 
in the current text... Perhaps simply adding @see ExecuteJob::newData to that 
enum's documentation might do the trick?

> kauthexecutejob.h:82
>      /**
> -     * @returns the data sent by the helper
> +     * @returns the data returned by the helper
>       */

Returned isn't semantically right, here, though the term "helper" is a bit 
ambiguous... What is meant by "sent by the helper" is more to the point "send 
by the helper using HelperSupport::progressStep(QVariant)"... which is somewhat 
clunky, but explains more properly what is going on here. Return would suggest 
that it is only done at the end, whereas this can, specifically, be done 
multiple times (basically, every time progressStep(QVariant) is called). So... 
perhaps changing this to be something like

  /**
   * Use this to get the data set by HelperSupport::progressStep(QVariant)
   * This function is particularly useful once the
   * job has completed. During execution, simply
   * read the data in the newData signal.
   * @see ExecuteJob::newData
   * returns the data sent by the helper
   */

REPOSITORY
  R283 KAuth

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

To: jriddell, leinir
Cc: #frameworks

Reply via email to