Review Request: Clean up rtm headers and move private data from Auth and Request classes into private classes. Also move webview from rtm library into plasma applet.

Jeremy Paul Whiting jpwhiting at kde.org
Wed Aug 15 00:00:40 UTC 2012



> On Aug. 2, 2012, 6:27 p.m., Andrew Stromme wrote:
> > libs/rtm/auth.cpp, lines 124-126
> > <http://git.reviewboard.kde.org/r/105734/diff/1-2/?file=74815#file74815line124>
> >
> >     The frob could be empty for two reasons:
> >     
> >        1. It wasn't requested
> >        2. There was some error in the request
> >     
> >     Because you call getAuthUrl() when the frob comes in I don't think it will ever be the situation that the frob is on its way but isn't there yet. I think in the case when there is no frob it might be best to return a Null QString() to indicate the error condition.

I added the while loop because I was hitting this case.  I request a frob when an auth is created in it's constructor now (otherwise it can't really do it's job), but the frob was empty when the AuthJob asked for the url, so I set it to wait until the frob was ready.  Maybe I should check if there's a frobRequest and send one right before waiting? to catch the case that there was an error or something?

Alternatively I could return an empty string and the AuthJob could wait for a url to get populated say 60 seconds or so then report a timeout to the ui?


- Jeremy Paul


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105734/#review16799
-----------------------------------------------------------


On Aug. 1, 2012, 2:45 p.m., Jeremy Paul Whiting wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105734/
> -----------------------------------------------------------
> 
> (Updated Aug. 1, 2012, 2:45 p.m.)
> 
> 
> Review request for Plasma and Andrew Stromme.
> 
> 
> Description
> -------
> 
> rtm: Move Request private data into RequestPrivate class in .cpp file. rtm: Move Auth private data into AuthPrivate class in .cpp file. rtm: Clean up auth.h and request.h files.
> 
> 
> rtm: Clean up rtm header files.
> 
> 
> Add #include <rtm/task.h> since we use Task * (and session.h has been cleaned up).
> 
> 
> Diffs
> -----
> 
>   applets/rememberthemilk/CMakeLists.txt 18424ead610b78710fa896a4ea3986fcd7e2f364 
>   applets/rememberthemilk/authenticate.ui cf7b9330d6d8f834225878f24eeb9be90651dbef 
>   applets/rememberthemilk/rememberthemilk-plasmoid.h cf02fbad4ab3e46d051d26b2de55c2ff01fb00b8 
>   applets/rememberthemilk/rememberthemilk-plasmoid.cpp 3413e59c69c079480efb2ba37e63927ad45df56e 
>   dataengines/rememberthemilk/authservice.h 31f44284422ea4ffdcc7631b5b321a1785480cdb 
>   dataengines/rememberthemilk/authservice.cpp c75e6cb6414920871358982aa226b3cbab1ca02f 
>   dataengines/rememberthemilk/rtmauth.operations 155a8ae6bdfde4d4fa1056ba0cc750ea6b9214e1 
>   dataengines/rememberthemilk/taskservice.cpp d738093ffd7c5183cf24f9f932d0c7f934b6634b 
>   libs/rtm/CMakeLists.txt 58ab4939812bbce07068a6da8c1a1194f6377595 
>   libs/rtm/auth.h 5f693fa53fc273e2a3782ac57e8c1ca3f8b194a1 
>   libs/rtm/auth.cpp 16fd5f0c28004dcc12140556e8d6727c483c2ed9 
>   libs/rtm/note.h af4cfd20c6ddaf1a4943f76f148c79918b5ec633 
>   libs/rtm/request.h d371714c144bf9bc6c7304a861840334f0846cdb 
>   libs/rtm/request.cpp 1304cb5d1a547d08b7f82e44cd319097991c8425 
>   libs/rtm/rtm.h 1a542f6007c63df13b79dd60da0cfcfe294f8e16 
>   libs/rtm/session.h aad145c44c70fb28fe1496b93d991667e918f630 
>   libs/rtm/session.cpp 051838f58c8f7a6b6aea9511f1d455197fafb08b 
>   libs/rtm/xmlreaders.h b9ea78ca567e32aceb70361a2735b7b005d15777 
> 
> Diff: http://git.reviewboard.kde.org/r/105734/diff/
> 
> 
> Testing
> -------
> 
> It builds and runs ok here (the rtm applet logs in and loads data ok).
> 
> 
> Thanks,
> 
> Jeremy Paul Whiting
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20120815/9f80d1aa/attachment.html>


More information about the Plasma-devel mailing list