cfeck added inline comments.
INLINE COMMENTS
> cfeck wrote in kpasswordlineedit.cpp:30
> Does this class need to be a QObject? If not, remove the QObject stuff.
Ping?
> kpasswordlineedit.h:53
> +Q_PROPERTY(bool clearButtonEnabled READ isClearButtonEnabled WRITE
> setClearButtonEnabled)
>
cfeck added inline comments.
INLINE COMMENTS
> elvisangelaccio wrote in knewpasswordwidget.cpp:118
> Then please rename the slot to `_k_echoModeToggled`, otherwise is confusing.
Ah, now I understand. Also, use the passed echoMode, instead of accessing
lineEdit().
REVISION DETAIL
davidedmundson added a comment.
Apparently doesn't fix the bug that brought me here.
To me the code still makes sense, but equally I don't want to touch this
clearly cursed class any more than possible.
REPOSITORY
R242 Plasma Framework (Library)
REVISION DETAIL
cfeck added inline comments.
INLINE COMMENTS
> cfeck wrote in knewpasswordwidget.cpp:75
> While you are at renaming slots: -> _k_passwordChanged
Ah wait, you are using the same slot for the verify widget, so maybe scratch
that comment.
REVISION DETAIL
https://phabricator.kde.org/D7011
To:
dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> chinmoyr wrote in filehelper.cpp:86
> In case of OPEN and OPENDIR, I can see why we need error checking, opendir()
> can return null and there are some
BUILD UNSTABLE
Build URL
https://build.kde.org/job/Frameworks%20bluez-qt%20kf5-qt5%20XenialQt5.7/16/
Project:
Frameworks bluez-qt kf5-qt5 XenialQt5.7
Date of build:
Thu, 03 Aug 2017 07:11:40 +
Build duration:
2 min 58 sec and counting
JUnit Tests
matank updated this revision to Diff 17609.
matank added a comment.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
- Fix issue where notifications will show as 1 pixel line
Sets the dialog to his correct size to prevent it showing
davidedmundson added a comment.
Can you explain the logic behind adding the code you've added.
REPOSITORY
R242 Plasma Framework (Library)
REVISION DETAIL
https://phabricator.kde.org/D7071
To: matank, #plasma, davidedmundson
Cc: plasma-devel, davidedmundson, ltoscano, #frameworks,
elvisangelaccio added inline comments.
INLINE COMMENTS
> mlaurent wrote in knewpasswordwidget.cpp:118
> Nope it's ok as code is called after changing echomode
> I created an autotest for testing it
Then please rename the slot to `_k_echoModeToggled`, otherwise is confusing.
REVISION DETAIL
matank added a comment.
Calling syncToMainItemSize() in componentComplete() seems to solve the issue
, so I checked what is being done in syncToMainItemSize() that affects the
issue.
I found that resizing the dialog is what seems to resolve the issue so I
copied the relevant code from
kfunk added inline comments.
INLINE COMMENTS
> cfeck wrote in kpasswordlineedit.cpp:30
> That's fishy. I just checked a single class (KColumnResizer), and it has just
> "class KColumnResizerPrivate" (no namespace, no QObject), and still has a
> Q_PRIVATE_SLOT in its KColumnResizer class.
+1.
mlaurent added inline comments.
INLINE COMMENTS
> cfeck wrote in kpasswordlineedit.cpp:30
> That's fishy. I just checked a single class (KColumnResizer), and it has just
> "class KColumnResizerPrivate" (no namespace, no QObject), and still has a
> Q_PRIVATE_SLOT in its KColumnResizer class.
mlaurent updated this revision to Diff 17615.
mlaurent added a comment.
Fix all comment reported
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D7011?vs=17606=17615
REVISION DETAIL
https://phabricator.kde.org/D7011
AFFECTED FILES
autotests/CMakeLists.txt
This revision was automatically updated to reflect the committed changes.
sitter marked an inline comment as done.
Closed by commit R290:ef15ef0889d2: kpackagetool can now write appstream data
to a file (authored by sitter).
CHANGED PRIOR TO COMMIT
BUILD UNSTABLE
Build URL
https://build.kde.org/job/Frameworks%20kirigami%20kf5-qt5%20FreeBSDQt5.7/26/
Project:
Frameworks kirigami kf5-qt5 FreeBSDQt5.7
Date of build:
Thu, 03 Aug 2017 11:20:24 +
Build duration:
48 sec and counting
JUnit Tests
BUILD UNSTABLE
Build URL
https://build.kde.org/job/Frameworks%20kirigami%20kf5-qt5%20XenialQt5.7/28/
Project:
Frameworks kirigami kf5-qt5 XenialQt5.7
Date of build:
Thu, 03 Aug 2017 11:20:24 +
Build duration:
1 min 9 sec and counting
JUnit Tests
cfeck added inline comments.
INLINE COMMENTS
> cfeck wrote in kpasswordlineedit.cpp:30
> Ping?
You made it a namespaced class again. Why?
REVISION DETAIL
https://phabricator.kde.org/D7011
To: mlaurent, cfeck, dfaure, elvisangelaccio
Cc: kossebau, elvisangelaccio, #frameworks
sitter marked an inline comment as done.
sitter added inline comments.
INLINE COMMENTS
> apol wrote in kpackagetool.cpp:473
> Why the `? true : false`?
lol, brain fart that.
REPOSITORY
R290 KPackage
BRANCH
master
REVISION DETAIL
https://phabricator.kde.org/D7069
To: sitter, apol,
mlaurent marked 4 inline comments as done.
mlaurent added inline comments.
INLINE COMMENTS
> cfeck wrote in knewpasswordwidget.cpp:118
> Ah, now I understand. Also, use the passed echoMode, instead of accessing
> lineEdit().
we can't as we use it in :
connect(toggleEchoModeAction, ::triggered,
cfeck added inline comments.
INLINE COMMENTS
> mlaurent wrote in kpasswordlineedit.cpp:30
> Otherwise it failed to using Q_PRIVATE_SLOT...
That's fishy. I just checked a single class (KColumnResizer), and it has just
"class KColumnResizerPrivate" (no namespace, no QObject), and still has a
BUILD UNSTABLE
Build URL
https://build.kde.org/job/Frameworks%20kpackage%20kf5-qt5%20XenialQt5.7/17/
Project:
Frameworks kpackage kf5-qt5 XenialQt5.7
Date of build:
Thu, 03 Aug 2017 10:15:14 +
Build duration:
2 min 20 sec and counting
JUnit Tests
BUILD UNSTABLE
Build URL
https://build.kde.org/job/Frameworks%20kirigami%20kf5-qt5%20XenialQt5.7/31/
Project:
Frameworks kirigami kf5-qt5 XenialQt5.7
Date of build:
Thu, 03 Aug 2017 14:42:46 +
Build duration:
52 sec and counting
JUnit Tests
BUILD UNSTABLE
Build URL
https://build.kde.org/job/Frameworks%20kirigami%20kf5-qt5%20FreeBSDQt5.7/29/
Project:
Frameworks kirigami kf5-qt5 FreeBSDQt5.7
Date of build:
Thu, 03 Aug 2017 14:42:46 +
Build duration:
26 sec and counting
JUnit Tests
brauch updated this revision to Diff 17638.
brauch added a comment.
Right. Better call it here.
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D7010?vs=17411=17638
REVISION DETAIL
https://phabricator.kde.org/D7010
AFFECTED FILES
src/ksqueezedtextlabel.cpp
To: brauch, cfeck
Cc:
BUILD UNSTABLE
Build URL
https://build.kde.org/job/Frameworks%20kirigami%20kf5-qt5%20XenialQt5.7/32/
Project:
Frameworks kirigami kf5-qt5 XenialQt5.7
Date of build:
Thu, 03 Aug 2017 14:49:51 +
Build duration:
58 sec and counting
JUnit Tests
BUILD UNSTABLE
Build URL
https://build.kde.org/job/Frameworks%20kirigami%20kf5-qt5%20FreeBSDQt5.7/31/
Project:
Frameworks kirigami kf5-qt5 FreeBSDQt5.7
Date of build:
Thu, 03 Aug 2017 15:30:46 +
Build duration:
1 min 8 sec and counting
JUnit Tests
BUILD UNSTABLE
Build URL
https://build.kde.org/job/Frameworks%20kirigami%20kf5-qt5%20XenialQt5.7/33/
Project:
Frameworks kirigami kf5-qt5 XenialQt5.7
Date of build:
Thu, 03 Aug 2017 15:30:46 +
Build duration:
1 min 0 sec and counting
JUnit Tests
cfeck accepted this revision.
cfeck added a comment.
Together with the Designer stuff, please.
REVISION DETAIL
https://phabricator.kde.org/D7011
To: mlaurent, cfeck, dfaure, elvisangelaccio
Cc: kfunk, kossebau, elvisangelaccio, #frameworks
cfeck removed a reviewer: elvisangelaccio.
This revision is now accepted and ready to land.
REVISION DETAIL
https://phabricator.kde.org/D7011
To: mlaurent, cfeck, dfaure
Cc: kfunk, kossebau, elvisangelaccio, #frameworks
BUILD UNSTABLE
Build URL
https://build.kde.org/job/Frameworks%20kirigami%20kf5-qt5%20FreeBSDQt5.7/27/
Project:
Frameworks kirigami kf5-qt5 FreeBSDQt5.7
Date of build:
Thu, 03 Aug 2017 14:15:24 +
Build duration:
48 sec and counting
JUnit Tests
apol created this revision.
Restricted Application added projects: Frameworks, Build System.
REVISION SUMMARY
Allows to check if a module is available on the system and sets it as a
runtime dependency.
This is useful for projects so that they can specify their qml dependencies
easily and
BUILD UNSTABLE
Build URL
https://build.kde.org/job/Frameworks%20kirigami%20kf5-qt5%20XenialQt5.7/29/
Project:
Frameworks kirigami kf5-qt5 XenialQt5.7
Date of build:
Thu, 03 Aug 2017 14:15:24 +
Build duration:
4 min 23 sec and counting
JUnit Tests
BUILD UNSTABLE
Build URL
https://build.kde.org/job/Frameworks%20kirigami%20kf5-qt5%20FreeBSDQt5.7/28/
Project:
Frameworks kirigami kf5-qt5 FreeBSDQt5.7
Date of build:
Thu, 03 Aug 2017 14:28:44 +
Build duration:
26 sec and counting
JUnit Tests
apol added a comment.
I just tried now. It works.
REPOSITORY
R288 KJobWidgets
REVISION DETAIL
https://phabricator.kde.org/D7058
To: apol, #frameworks
Cc: aacid
BUILD UNSTABLE
Build URL
https://build.kde.org/job/Frameworks%20kirigami%20kf5-qt5%20XenialQt5.7/30/
Project:
Frameworks kirigami kf5-qt5 XenialQt5.7
Date of build:
Thu, 03 Aug 2017 14:28:44 +
Build duration:
4 min 11 sec and counting
JUnit Tests
cfeck requested changes to this revision.
cfeck added a comment.
This revision now requires changes to proceed.
The title says "when text changes", but you call it even if the text does not
change. Note that squeezeTextToLabel() is called from many places, including
resizeEvent(), so avoid
vkrause created this revision.
Restricted Application added a project: Frameworks.
REPOSITORY
R216 Syntax Highlighting
BRANCH
hl-cross-compile
REVISION DETAIL
https://phabricator.kde.org/D7102
AFFECTED FILES
src/indexer/CMakeLists.txt
To: vkrause, #frameworks
kfunk accepted this revision.
kfunk added a comment.
This revision is now accepted and ready to land.
TIL you can use `IMPORTED` on `add_executable`...
LGTM
REPOSITORY
R216 Syntax Highlighting
BRANCH
hl-cross-compile
REVISION DETAIL
https://phabricator.kde.org/D7102
To: vkrause,
vkrause created this revision.
Restricted Application added a project: Frameworks.
REVISION SUMMARY
By default, Qt is built without accessibility there, which is what
would indirectly include the needed QColor with a "normal" Qt build
here.
REPOSITORY
R242 Plasma Framework (Library)
kfunk accepted this revision.
kfunk added inline comments.
This revision is now accepted and ready to land.
INLINE COMMENTS
> CMakeLists.txt:3
> +if(TARGET Qt5::Gui)
> +add_subdirectory(lib)
> +add_subdirectory(cli)
Too bad it needs QtGui. Just b/c it uses `QColor` as far as I can
mlaurent closed this revision.
REVISION DETAIL
https://phabricator.kde.org/D7062
To: mlaurent, dfaure, kossebau, cfeck
Cc: cfeck, elvisangelaccio, #frameworks
This revision was automatically updated to reflect the committed changes.
Closed by commit R242:0a8d1f9cd474: Fix compilation on Yocto (authored by
vkrause).
REPOSITORY
R242 Plasma Framework (Library)
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D7117?vs=17661=17677
REVISION
broulik accepted this revision.
This revision is now accepted and ready to land.
REPOSITORY
R242 Plasma Framework (Library)
BRANCH
master
REVISION DETAIL
https://phabricator.kde.org/D7117
To: vkrause, #frameworks, broulik
cfeck added a comment.
Are you sure the patch is complete? I remember there was code which was
supposed to be replaced by the new validator.
INLINE COMMENTS
> ktimecombobox.cpp:608
> #include "moc_ktimecombobox.cpp"
> +#include "ktimecombobox.moc"
This does not look right.
REPOSITORY
cfeck requested changes to this revision.
cfeck added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> ktimecombobox.cpp:33
> +public:
> +KTimeValidator(QString timeFormat, QObject *parent = Q_NULLPTR);
> +KTimeValidator(QString timeFormat, QTime min,
elvisangelaccio added a comment.
+1, looks good to me now.
REVISION DETAIL
https://phabricator.kde.org/D7011
To: mlaurent, cfeck, dfaure, elvisangelaccio
Cc: kfunk, kossebau, elvisangelaccio, #frameworks
cfeck added inline comments.
INLINE COMMENTS
> kpasswordlineedit.cpp:3
> + Copyright (c) 2017 Montel Laurent
> +
> + This library is free software; you can redistribute it and/or modify
Elvis, you wrote the "visibilityAction" stuff, right? If yes, needs to be
mentioned
cfeck added inline comments.
INLINE COMMENTS
> kfunk wrote in kpasswordlineedit.cpp:30
> +1. Don't use nested private classes. Nested classes/structs inherit the
> symbols visibility.
>
> Also see: https://phabricator.kde.org/D6927
>
> PS: In times of Qt5 & lambda-connects, you don't need
mlaurent updated this revision to Diff 17621.
mlaurent added a comment.
+1
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D7011?vs=17620=17621
REVISION DETAIL
https://phabricator.kde.org/D7011
AFFECTED FILES
autotests/CMakeLists.txt
autotests/knewpasswordwidgettest.cpp
cfeck added a comment.
Looks good to go from my side. Let's wait a bit if someone has more
objections.
Thanks Laurent, it was a needed change considering the code duplication that
was present before.
REVISION DETAIL
https://phabricator.kde.org/D7011
To: mlaurent, cfeck, dfaure,
50 matches
Mail list logo