<table><tr><td style="">leinir requested changes to this revision.<br />leinir added a comment.<br />This revision now requires changes to proceed.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D5003" rel="noreferrer">View Revision</a></tr></table><br /><div><div><p>Thinking this is... well under way, with a couple of details :)</p></div></div><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D5003#inline-20436" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">kauthactionreply.h:338</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span style="color: #74777d">*</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span style="color: #74777d">* In case of errors coming from the library, the type() is</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span style="color: #74777d">* ActionReply::KAuthError. In this case, errorCode() will always be one of the</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">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 <span class="phabricator-remarkup-mention-unknown">@see</span> ExecuteJob::newData to that enum's documentation might do the trick?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D5003#inline-20435" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">kauthexecutejob.h:82</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; "> <span style="color: #74777d">/**</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span style="color: #74777d"> * @returns the data <span class="bright">sent</span> by the helper</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d"> * @returns the data <span class="bright">returned</span> by the helper</span>
</div><div style="padding: 0 8px; margin: 0 4px; "><span style="color: #74777d"> */</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">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</p>
<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">/**
* 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
*/</pre></div></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R283 KAuth</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D5003" rel="noreferrer">https://phabricator.kde.org/D5003</a></div></div><br /><div><strong>To: </strong>jriddell, leinir<br /><strong>Cc: </strong>Frameworks<br /></div>