Review Request 129653: Fix KAuth helper error code reporting to match documentation

[Aleix Pol Gonzalez]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129653/#review101453
-----------------------------------------------------------



+1 the patch makes sense.


src/kauthactionreply.h (line 676)
<https://git.reviewboard.kde.org/r/129653/#comment67924>

    There's `@param` but the argument doesn't have a name.


- Aleix Pol Gonzalez


On Dec. 15, 2016, 2:52 a.m., David Edmundson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129653/
> -----------------------------------------------------------
> 
> (Updated Dec. 15, 2016, 2:52 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kauth
> 
> 
> Description
> -------
> 
> In KDElibs4 KAuth::ActionReply::errorCode returned an int.
> 
> errorCode was documented as
> 
> "The error code returned is one of the values in the ActionReply::Error
> enumeration if type() == KAuthError, or is totally application-dependent
> if
> type() == HelperError. It also should be zero for successful replies."
> 
> In frameworks that documentation is exactly the same, however
> setErrorCode and errorCode were changed to only use
> the enum values in ActionReply::Error as an argument, which goes against
> the documentation and is only meant to be used for internal errors, not
> KAuth helpers.
> 
> Worst this leads to broken error reporting, because if one uses the new
> HelperError static constructor, we have type==HelperError, but errorCode
> is still 0.
> This means ActionReply::failed is true, but at a KJob level, we think
> the action succeeded.
> 
> Some clients are working round this by either static casting or by
> incorrectly using the ActionReply::Error codes which are meant purely
> for backend errors not helper errors.
> Others are simply failing to report errors.
> 
> We need a fix that will support the clients which are passing an error
> code back, and also fix clients which are just using
> KAuth::HelperError()
> 
> This patch reintroduces the method as an integer, but changing the name
> for compatibility reasons. Also a new static constructor is created that
> allows one to set an error code when the AuthReply type is HelperError.
> 
> 
> Diffs
> -----
> 
>   autotests/HelperTest.cpp 176d3ccb487c07e906b4834f61e7a4add0695713 
>   autotests/TestHelper.h 214524352adbc647c586776caf6f76f587ca8001 
>   autotests/TestHelper.cpp 0302dfaa889ba6dff03472a4049255410358c38a 
>   src/kauthactionreply.h 5d057bbd1d584d24b3fd838e0eaf2ea342743b0a 
>   src/kauthactionreply.cpp 0309eaa932155bade00c1413ade3bfc983dbf307 
> 
> Diff: https://git.reviewboard.kde.org/r/129653/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20161215/026334df/attachment.html>