[KDE/Mac] Review Request 119844: [OS X] backend improvements for kwallet/Keychain integration

[Albert Astals Cid]

> On set. 5, 2014, 5:52 p.m., Marko Käning wrote:
> > tests/KWallet/kwallettest.cpp, line 53
> > <https://git.reviewboard.kde.org/r/119844/diff/4/?file=309940#file309940line53>
> >
> >     Are whitespace changes like this introduced by running the sources through krazy and thus comply with the KDE coding style now?
> 
> René J.V. Bertin wrote:
>     No and I have no idea. This is part of a coding style that is (easily, quickly) readable to/by me ...
> 
> Marko Käning wrote:
>     Hmm... Well, I am afraid that's not accepted then. You need to run krazy on the sources and stick to the KDE coding style.
> 
> Ian Wadham wrote:
>     Indeed. There are two firm conventions in KDE.
>     
>     One is that library code must adhere to the standard KDE style. The other is that you can use your own style in applications code, BUT, if you are modifying someone else's original code, you must stick to his or her style. These conventions improve readability for the KDE group as a whole and might also make life easier for scripts that trawl through the code from time to time.
> 
> Marko Käning wrote:
>     Wondering whether following https://gitorious.org/krazy/krazy/source/b7c763888c6d4538e73675d2b7255e0e6712b4df:INSTALL.txt will allow to successfully install this on René's system.
> 
> René J.V. Bertin wrote:
>     As I said (but apparently not here) I'm not particularly motivated (not to say I'd be {c,k}razy) to jump through hoops for something as trivial as slight whitespace differences ... especially if it's to reduce readability to/in my eyes ;)
>     
>     But just for the record: this something that could be done as a final step just before or after a Ship It?
>     
>     And out of curiosity: when was it decided that it's a good idea to put the space between an if/while/etc and the bracket and none after the bracket instead of the other way round (or at least no space before the bracket)? I'm pretty sure that wasn't the convention when I learned C in the late 80s, just as it still isn't for functions. I find it semantically confusing to say the least; it suggests that the brackets aren't required (just as they aren't in Python or Pascal style languages' if/while/etc expressions), but that's still not true AFAIK.
> 
> Marko Käning wrote:
>     I see your point(s). :) Of course, running krazy can be done as the last step.

Please, don't do any whitespace change in existing code, it makes reviewing harder because the reviewer has to think if something really changed on the line or is it that someone beauty standards are different from the past guy beauty standards.


- Albert


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


On set. 5, 2014, 4:21 p.m., René J.V. Bertin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119844/
> -----------------------------------------------------------
> 
> (Updated set. 5, 2014, 4:21 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and Valentin Rusu.
> 
> 
> Repository: kwallet
> 
> 
> Description
> -------
> 
> The submitted improvements to kwallet consist of a number of changes to existing files, as well as 2 new files that contain the actual interface to OS X's SecKeychain API.
> 
> With these modifications, KDE wallets are stored in the same location as native OS X keychains, and both can be managed (up to a certain extent) in the OS X Keychain utility as well as the kwalletmanager. In addition, password prompts no longer get posted somewhere in the background.
> 
> 
> Diffs
> -----
> 
>   src/api/KWallet/CMakeLists.txt 9709559 
>   src/api/KWallet/kwallet_mac.cpp d93e5ae 
>   src/api/KWallet/qosxkeychain.h PRE-CREATION 
>   src/api/KWallet/qosxkeychain.cpp PRE-CREATION 
>   tests/KWallet/CMakeLists.txt b155f64 
>   tests/KWallet/kwallettest.cpp 3351a6b 
> 
> Diff: https://git.reviewboard.kde.org/r/119844/diff/
> 
> 
> Testing
> -------
> 
> Testing and development of these was done on OS X 10.6.8 running KDE 4.12.5, which is part of my production environment (https://trac.macports.org/ticket/44473). Since I am not currently able to build (parts of) KF5, porting of my modifications to KF5/KWallet has been done in source only. However, I have good hope that there will be little bugs to review in this request, given the lack of non-esthetic (formatting) modifications to the current kwallet_mac.cpp and kwallettest.cpp.
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-mac/attachments/20140908/162221c6/attachment.html>