> On May 6, 2013, 4:40 p.m., Lamarque Souza wrote:
> > kwalletd/kwalletd.h, line 226
> > <http://git.reviewboard.kde.org/r/110328/diff/1/?file=142372#file142372line226>
> >
> >     Why split this feature into three different review requests? One of 
> > your requests [2] is just one line change and the other [1] is not visible 
> > from reviewboard. I think you should merge those two requests below with 
> > this one.
> >     
> >     [1] https://git.reviewboard.kde.org/r/110330
> >     [2] https://git.reviewboard.kde.org/r/110331

110328 and 110330 are for independent features that need to be reviewed 
independently, and 110331 is for a different repository (kwallet.git vs 
kde-runtime.git).


> On May 6, 2013, 4:40 p.m., Lamarque Souza wrote:
> > kwalletd/kwalletd.cpp, line 506
> > <http://git.reviewboard.kde.org/r/110328/diff/1/?file=142373#file142373line506>
> >
> >     password is a QString, right? Then you should use password.clear() here 
> > instead of assigning an empty QString. That avoids creating a temporary 
> > QString.

If you look at the code later, it cares whether password is null or empty via 
an isNull() check. An empty QString is different from a null one.


- Eike


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


On May 6, 2013, 4:01 p.m., Eike Hein wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110328/
> -----------------------------------------------------------
> 
> (Updated May 6, 2013, 4:01 p.m.)
> 
> 
> Review request for KDE Runtime and Harald Sitter.
> 
> 
> Description
> -------
> 
> This patch adds a UI-less config option to kwalletd that makes it create the 
> initial local wallet silently with an empty password instead of prompting the 
> user to enter one.
> 
> It's a change desired by downstream consumers Kubuntu and Netrunner, and 
> perhaps others, and recreates a modification they used to carry for KDE 3. 
> Their goal is to make KWallet mostly invisible to the user during routine 
> operations, but still have users benefit from encrypted password storage 
> behind the scenes.
> 
> As such the config option is intended to be set by distributions. The new 
> behavior is disabled by default.
> 
> In the interest of keeping the delta between upstream and downstream as small 
> as possible I'd say it makes sense to pick this up.
> 
> 
> Diffs
> -----
> 
>   kwalletd/kwalletd.h e8e74c3 
>   kwalletd/kwalletd.cpp fa9fc11 
> 
> Diff: http://git.reviewboard.kde.org/r/110328/diff/
> 
> 
> Testing
> -------
> 
> Test package for Kubuntu by Harald Sitter, operation verified at runtime.
> 
> 
> Thanks,
> 
> Eike Hein
> 
>

Reply via email to