kossebau added inline comments.
INLINE COMMENTS
> kpasswordlineedit.cpp:30
> +
> +class KPasswordLineEdit::KPasswordLineEditPrivate : public QObject
> +{
Make this a normal class KPasswordLineEditPrivate here, following fixes
proposed above for the header.
> kpasswordlineedit.h:25
> +class
mlaurent updated this revision to Diff 17469.
mlaurent added a comment.
I renamed class/test apps/autotests as requested
I fixed autotest with new autotest from master
I fixed all comment on this review
etc.
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D7011?vs=17420=17469
mlaurent marked 7 inline comments as done.
REVISION DETAIL
https://phabricator.kde.org/D7011
To: mlaurent, cfeck, dfaure, elvisangelaccio
Cc: kossebau, elvisangelaccio, #frameworks
apol accepted this revision.
apol added a comment.
This revision is now accepted and ready to land.
That path changed since the `--gradle` change, commit together.
REPOSITORY
R240 Extra CMake Modules
BRANCH
fix_install_apk
REVISION DETAIL
https://phabricator.kde.org/D6876
To: aacid,
apol accepted this revision.
apol added a comment.
This revision is now accepted and ready to land.
Fixes the issue for me too.
REPOSITORY
R240 Extra CMake Modules
BRANCH
add_gradle
REVISION DETAIL
https://phabricator.kde.org/D6875
To: aacid, apol, kossebau, mart
Cc: #frameworks,
ltoscano requested review of this revision.
ltoscano added a comment.
Now the patch should be more complete and allows its usage in translated
documentation.
REPOSITORY
R238 KDocTools
REVISION DETAIL
https://phabricator.kde.org/D6972
To: ltoscano, jriddell, lueck
Cc: jriddell, lueck,
ltoscano updated this revision to Diff 17466.
ltoscano added a comment.
Make the entity usable for translated documents
REPOSITORY
R238 KDocTools
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D6972?vs=17451=17466
BRANCH
add-ccbysa4-translatable
REVISION DETAIL
kossebau added inline comments.
INLINE COMMENTS
> mlaurent wrote in knewpasswordwidgettest.cpp:63
> Nope as setPassword not authorize to see icon (it's a security when we
> setPassword from apps you don't want to see it)
Could you add this as comment to this line? it is not directly obvious,
ltoscano added inline comments.
INLINE COMMENTS
> general.entities:57
>"../en/entities/lgpl-notice.docbook">
> + + "-//KDE//DOCUMENT CC BY-SA 4.0 License Notice//EN"
Rereading the code, I was wondering if I should make this translatable (aka
defined into en/entities like FDLNotice).
lueck accepted this revision.
lueck added a comment.
This revision is now accepted and ready to land.
thanks
REPOSITORY
R238 KDocTools
BRANCH
add-ccbysa4
REVISION DETAIL
https://phabricator.kde.org/D6972
To: ltoscano, jriddell, lueck
Cc: jriddell, lueck, #frameworks, #documentation,
bednar updated this revision to Diff 17452.
bednar added a comment.
Ran through uncrustify.
Found a regression in my patch : colon (":") wouldn't be shown anymore, as it
is dependent on the input mask. So I left the input mask, and the QValidator
works in addition to the input mask.
ltoscano requested review of this revision.
REPOSITORY
R238 KDocTools
REVISION DETAIL
https://phabricator.kde.org/D6972
To: ltoscano, jriddell
Cc: jriddell, lueck, #frameworks, #documentation, skadinna
ltoscano retitled this revision from "Add CC BY-SA 4.0 International" to "Add
CC BY-SA 4.0 International and set it as default".
REPOSITORY
R238 KDocTools
BRANCH
add-ccbysa4
REVISION DETAIL
https://phabricator.kde.org/D6972
To: ltoscano, jriddell
Cc: jriddell, lueck, #frameworks,
ltoscano updated this revision to Diff 17451.
ltoscano added a comment.
This revision is now accepted and ready to land.
Address the comments
REPOSITORY
R238 KDocTools
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D6972?vs=17339=17451
BRANCH
add-ccbysa4
REVISION DETAIL
mlaurent marked 5 inline comments as done.
REVISION DETAIL
https://phabricator.kde.org/D7011
To: mlaurent, cfeck, dfaure, elvisangelaccio
Cc: kossebau, elvisangelaccio, #frameworks
mlaurent added inline comments.
INLINE COMMENTS
> cfeck wrote in knewpasswordwidgettest.cpp:63
> Does using setPassword() not work for these tests?
Nope as setPassword not authorize to see icon (it's a security when we
setPassword from apps you don't want to see it)
> cfeck wrote in
ltoscano planned changes to this revision.
ltoscano added a comment.
I need to apply the changes requested by Burkhard first.
REPOSITORY
R238 KDocTools
REVISION DETAIL
https://phabricator.kde.org/D6972
To: ltoscano, jriddell
Cc: jriddell, lueck, #frameworks, #documentation, skadinna
jriddell accepted this revision.
jriddell added a comment.
This revision is now accepted and ready to land.
thanks
REPOSITORY
R238 KDocTools
BRANCH
add-ccbysa4
REVISION DETAIL
https://phabricator.kde.org/D6972
To: ltoscano, jriddell
Cc: jriddell, lueck, #frameworks, #documentation,
cfeck requested changes to this revision.
REVISION DETAIL
https://phabricator.kde.org/D7011
To: mlaurent, cfeck, dfaure, elvisangelaccio
Cc: elvisangelaccio, #frameworks
cfeck added a comment.
Any reason it should be PasswordLineEdit, and not KPasswordLineEdit? I do not
care about names, but I care about consistency.
INLINE COMMENTS
> knewpasswordwidgettest.cpp:63
> const QString password = QStringLiteral("1234");
> -
elvisangelaccio requested changes to this revision.
elvisangelaccio added a comment.
This revision now requires changes to proceed.
I spotted one small regression, I added a test case for it in
https://phabricator.kde.org/R236:380b35439a39950769fcca63f7ec31cc6170707a.
For manual test you
BUILD UNSTABLE
Build URL
https://build.kde.org/job/Frameworks%20extra-cmake-modules%20kf5-qt5%20XenialQt5.7/32/
Project:
Frameworks extra-cmake-modules kf5-qt5 XenialQt5.7
Date of build:
Mon, 31 Jul 2017 11:57:37 +
Build duration:
6 min 53 sec and counting
BUILD UNSTABLE
Build URL
https://build.kde.org/job/Frameworks%20extra-cmake-modules%20kf5-qt5%20FreeBSDQt5.7/41/
Project:
Frameworks extra-cmake-modules kf5-qt5 FreeBSDQt5.7
Date of build:
Mon, 31 Jul 2017 11:57:37 +
Build duration:
4 min 27 sec and counting
This revision was automatically updated to reflect the committed changes.
Closed by commit R240:899519f0be03: Make ECMPoQmToolsTest actually fail if a
translation is wrong (authored by wbauer).
REPOSITORY
R240 Extra CMake Modules
CHANGES SINCE LAST UPDATE
mlaurent updated this revision to Diff 17420.
mlaurent added a comment.
I renamed class name.
I ported KNewPasswordWidget .
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D7011?vs=17413=17420
REVISION DETAIL
https://phabricator.kde.org/D7011
AFFECTED FILES
vandenoever added a comment.
Removing this call is in line with the advice in the dbus spec.
An improvement to the patch would be to remove `bool startfdo`.
Q_WS_WIN seems to suggest that windows always needs to start the service
manually. That case would break with this patch.
elvisangelaccio added a comment.
Can you try to use this new widget also in KNewPasswordWidget?
I'm also not sure about the name. I think PasswordLineEdit would be more
clear? (since this is a line edit, not a password).
REPOSITORY
R236 KWidgetsAddons
REVISION DETAIL
vkrause added a comment.
Fair point, I'll add options for this.
REPOSITORY
R246 Sonnet
REVISION DETAIL
https://phabricator.kde.org/D6990
To: vkrause, #frameworks, cordlandwehr
Cc: aacid
davidedmundson added a comment.
No, his one still blocks. This doesn't.
REPOSITORY
R289 KNotifications
REVISION DETAIL
https://phabricator.kde.org/D6963
To: davidedmundson
Cc: mck182, dvratil, #frameworks
29 matches
Mail list logo
