D8810: Do not look for kioslave binary in applicationDirPath on *nix (#386859)

cullmann added a comment. I can live with that patch, thought I still think we should just rename the helper, but perhaps that breaks other things if people relied on the name. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8810 To: kkofler, #frameworks, kfunk,

D8396: Add support for zwp_idle_inhibit_manager_v1

graesslin added a comment. In https://phabricator.kde.org/D8396#168165, @davidedmundson wrote: > I think we need some big discussion about how powerdevil inhbitions, logind inhibitions and this are all going to fit together in a clear coherent way. yeah. I'm not happy with this

D8396: Add support for zwp_idle_inhibit_manager_v1

davidedmundson accepted this revision. davidedmundson added a comment. This revision is now accepted and ready to land. I think we need some big discussion about how powerdevil inhbitions, logind inhibitions and this are all going to fit together in a clear coherent way. But given there

D8811: [knewstuff] Do not leak ImageLoader on error

anthonyfieroni updated this revision to Diff 22408. REPOSITORY R304 KNewStuff CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8811?vs=22391=22408 REVISION DETAIL https://phabricator.kde.org/D8811 AFFECTED FILES src/core/engine.cpp src/core/imageloader.cpp

Re: How is symbol visibility set in KF5 and KDE?

On Wednesday, 15 November 2017 15:54:17 CET Shaheed Haque wrote: > Hi all, > > I just realised that the Python binding effort is not setting the default > visibility for symbols using the -fvisibility=xxx option when processing > the header files [1]. Of course I can see the export macros set by

D8159: Add API for setting server decoration palettes

davidedmundson planned changes to this revision. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D8159 To: davidedmundson, #plasma, graesslin Cc: graesslin, broulik, plasma-devel, #frameworks, leezu, ZrenBot, alexeymin, progwolff, lesliezhai, ali-mohamed,

How is symbol visibility set in KF5 and KDE?

Hi all, I just realised that the Python binding effort is not setting the default visibility for symbols using the -fvisibility=xxx option when processing the header files [1]. Of course I can see the export macros set by the likes of attica_exports.h, but I don't see where the compiler default

D8332: Added baloo urls into places model

mwolff added a comment. lgtm, one minor nit, potentially for the future INLINE COMMENTS > kfileplacesmodel.cpp:967 > > +bool KFilePlacesModel::Private::isFileIndexingEnabled() const > +{ this could/should be a free function, not a member, considering its result is cached in a member

D8348: Add a section for removable devices

mwolff accepted this revision. mwolff added a comment. This revision is now accepted and ready to land. lgtm too REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8348 To: renatoo, #dolphin, #frameworks, #vdg, ervin, ngraham, mwolff Cc: mwolff, abetts, mlaurent,

KDE CI: Frameworks kirigami kf5-qt5 XenialQt5.7 - Build # 151 - Fixed!

BUILD SUCCESS Build URL https://build.kde.org/job/Frameworks%20kirigami%20kf5-qt5%20XenialQt5.7/151/ Project: Frameworks kirigami kf5-qt5 XenialQt5.7 Date of build: Wed, 15 Nov 2017 13:55:33 + Build duration: 1 min 7 sec and counting JUnit Tests

KDE CI: Frameworks kirigami kf5-qt5 FreeBSDQt5.7 - Build # 145 - Still Unstable!

BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks%20kirigami%20kf5-qt5%20FreeBSDQt5.7/145/ Project: Frameworks kirigami kf5-qt5 FreeBSDQt5.7 Date of build: Wed, 15 Nov 2017 13:55:01 + Build duration: 47 sec and counting JUnit Tests

KDE CI: Frameworks kirigami kf5-qt5 XenialQt5.7 - Build # 150 - Unstable!

BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks%20kirigami%20kf5-qt5%20XenialQt5.7/150/ Project: Frameworks kirigami kf5-qt5 XenialQt5.7 Date of build: Wed, 15 Nov 2017 13:53:57 + Build duration: 1 min 32 sec and counting JUnit Tests

KDE CI: Frameworks kirigami kf5-qt5 FreeBSDQt5.7 - Build # 144 - Still Unstable!

BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks%20kirigami%20kf5-qt5%20FreeBSDQt5.7/144/ Project: Frameworks kirigami kf5-qt5 FreeBSDQt5.7 Date of build: Wed, 15 Nov 2017 13:53:57 + Build duration: 54 sec and counting JUnit Tests

D8348: Add a section for removable devices

ngraham accepted this revision. ngraham added a comment. In https://phabricator.kde.org/D8348#167982, @renatoo wrote: > This is not related with the change. I am working on that in a different branch. OK, thanks! Please add me to the review list for that patch, once it's up.

D8820: Don't notify about value changes if we are still in the constructor

This revision was automatically updated to reflect the committed changes. Closed by commit R169:09efedec8c71: Dont notify about value changes if we are still in the constructor (authored by apol). REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE

D8348: Add a section for removable devices

renatoo added inline comments. INLINE COMMENTS > mwolff wrote in kfileplacesitem.cpp:91 > shouldn't this always be called? i.e. when the bookmark is changed to a > different UDI, don't we need to update here, even when we had a valid device > UDI before? I think this also means this path isn't

D8348: Add a section for removable devices

renatoo marked an inline comment as done. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8348 To: renatoo, #dolphin, #frameworks, #vdg, ervin, ngraham, mwolff Cc: mwolff, abetts, mlaurent, anthonyfieroni, ngraham, #frameworks

D8348: Add a section for removable devices

renatoo updated this revision to Diff 22393. renatoo added a comment. Update device info if udi changes. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8348?vs=22329=22393 REVISION DETAIL https://phabricator.kde.org/D8348 AFFECTED FILES

D8434: Created 'remote' section

renatoo updated this revision to Diff 22394. renatoo added a comment. Updated parent branch REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8434?vs=22330=22394 REVISION DETAIL https://phabricator.kde.org/D8434 AFFECTED FILES

KDE CI: Frameworks kio kf5-qt5 FreeBSDQt5.7 - Build # 139 - Still Unstable!

BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20FreeBSDQt5.7/139/ Project: Frameworks kio kf5-qt5 FreeBSDQt5.7 Date of build: Wed, 15 Nov 2017 12:57:02 + Build duration: 24 min and counting JUnit Tests Name:

KDE CI: Frameworks kio kf5-qt5 XenialQt5.7 - Build # 138 - Still Unstable!

BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20XenialQt5.7/138/ Project: Frameworks kio kf5-qt5 XenialQt5.7 Date of build: Wed, 15 Nov 2017 12:57:02 + Build duration: 20 min and counting JUnit Tests Name: (root)

D8820: Don't notify about value changes if we are still in the constructor

davidedmundson accepted this revision. This revision is now accepted and ready to land. REPOSITORY R169 Kirigami BRANCH master REVISION DETAIL https://phabricator.kde.org/D8820 To: apol, #frameworks, davidedmundson Cc: davidedmundson, broulik, plasma-devel, apol, mart, hein

D8811: [knewstuff] Do not leak ImageLoader on error

leinir added inline comments. INLINE COMMENTS > anthonyfieroni wrote in engine.cpp:536 > Job can be nullptr, so signal can be a text, it can capture entry by > reference? i guess you could extend the signal further with the error text added in instead of capturing l in the lambda, yes

D8811: [knewstuff] Do not leak ImageLoader on error

anthonyfieroni added inline comments. INLINE COMMENTS > leinir wrote in engine.cpp:536 > You did add l to the capture, but unless i'm missing something super > obvious... you're not using it ;) i meant to do something like add `<< > l->job()->errorText()` to the debug statement so we can see

D8811: [knewstuff] Do not leak ImageLoader on error

leinir added inline comments. INLINE COMMENTS > engine.cpp:536 > +connect(l, ::signalError, this, [this, l](const > KNSCore::EntryInternal , EntryInternal::PreviewType type) { > +qCDebug(KNEWSTUFFCORE) << "ERROR preview: " << entry.name() << type; > +--m_numPictureJobs; You

D8811: [knewstuff] Do not leak ImageLoader on error

anthonyfieroni updated this revision to Diff 22391. REPOSITORY R304 KNewStuff CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8811?vs=22371=22391 REVISION DETAIL https://phabricator.kde.org/D8811 AFFECTED FILES src/core/engine.cpp src/core/imageloader.cpp

D8811: [knewstuff] Do not leak ImageLoader on error

anthonyfieroni marked an inline comment as done. REPOSITORY R304 KNewStuff REVISION DETAIL https://phabricator.kde.org/D8811 To: anthonyfieroni, leinir, dfaure Cc: broulik, #frameworks, ZrenBot

D8820: Don't notify about value changes if we are still in the constructor

apol added a comment. > If you're emitting something in the constructor, how could a UI be connected to it? There's a timer. REPOSITORY R169 Kirigami REVISION DETAIL https://phabricator.kde.org/D8820 To: apol, #frameworks Cc: davidedmundson, broulik, plasma-devel, apol, mart, hein

D8811: [knewstuff] Do not leak ImageLoader on error

leinir added inline comments. INLINE COMMENTS > engine.cpp:536 > +connect(l, ::signalError, this, [=]() { > +qCDebug(KNEWSTUFFCORE) << "ERROR preview: " << entry.name() << type; > +--m_numPictureJobs; A more descriptive error message would be good, but this is already an

D8820: Don't notify about value changes if we are still in the constructor

davidedmundson added a comment. > it will make the UI react to it at some point If you're emitting something in the constructor, how could a UI be connected to it? REPOSITORY R169 Kirigami REVISION DETAIL https://phabricator.kde.org/D8820 To: apol, #frameworks Cc: davidedmundson,

D8348: Add a section for removable devices

renatoo marked 2 inline comments as done. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8348 To: renatoo, #dolphin, #frameworks, #vdg, ervin, ngraham, mwolff Cc: mwolff, abetts, mlaurent, anthonyfieroni, ngraham, #frameworks

D8825: do not show edit bookmarks action if keditbookmarks is not installed

sitter added inline comments. INLINE COMMENTS > apol wrote in kbookmarkmenu_p.h:42 > Why not in the cpp file? It's used in 2 difference cpps. REPOSITORY R294 KBookmarks REVISION DETAIL https://phabricator.kde.org/D8825 To: sitter, #frameworks Cc: apol

D8348: Add a section for removable devices

renatoo added a comment. In https://phabricator.kde.org/D8348#167924, @mwolff wrote: > In https://phabricator.kde.org/D8348#163546, @ngraham wrote: > > > Could you add camera:/ devices to this section too? That way we could also take care of

D8825: do not show edit bookmarks action if keditbookmarks is not installed

apol added a comment. LGTM INLINE COMMENTS > kbookmarkmenu_p.h:42 > > +#define KEDITBOOKMARKS_BINARY "keditbookmarks" > + Why not in the cpp file? REPOSITORY R294 KBookmarks REVISION DETAIL https://phabricator.kde.org/D8825 To: sitter, #frameworks Cc: apol

D8824: port from deprecated KAuthorized::authorizeKAction to authorizeAction

apol accepted this revision. This revision is now accepted and ready to land. REPOSITORY R294 KBookmarks BRANCH kauthorize-deprecations REVISION DETAIL https://phabricator.kde.org/D8824 To: sitter, #frameworks, apol

D8450: User can now hide an entire places group from KFilePlacesView

mwolff requested changes to this revision. mwolff added a comment. This revision now requires changes to proceed. please use arc, or add more context to your patches in the future INLINE COMMENTS > kfileplacesview.cpp:298 > + > +m_disappearingItems.reserve(m_disappearingItems.count() +

D8367: Hidding place groups implementation in KFilePlacesModel

mwolff requested changes to this revision. mwolff added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > kfileplacesmodel.cpp:70 > +{ > +{ KFilePlacesModel::PlacesType, > QStringLiteral("GroupState-Places-IsHidden") }, > +{

D8348: Add a section for removable devices

mwolff added inline comments. INLINE COMMENTS > mwolff wrote in kfileplacesmodeltest.cpp:181 > the changed list of remote urls that is repeated below could be put into > another helper function done already in https://phabricator.kde.org/D8366 I see, ignore this REPOSITORY R241 KIO

D8348: Add a section for removable devices

mwolff requested changes to this revision. mwolff added a comment. This revision now requires changes to proceed. the UDI comment needs to be investigated, otherwise lgtm INLINE COMMENTS > kfileplacesmodeltest.cpp:181 > return QStringList() << QDir::homePath() <<

D8348: Add a section for removable devices

mwolff added a comment. In https://phabricator.kde.org/D8348#163546, @ngraham wrote: > Could you add camera:/ devices to this section too? That way we could also take care of https://bugs.kde.org/show_bug.cgi?id=386060 this isn't done yet, I think REPOSITORY R241 KIO REVISION

D8348: Add a section for removable devices

mwolff added a comment. In https://phabricator.kde.org/D8348#164718, @ngraham wrote: > In https://phabricator.kde.org/D8348#164713, @abetts wrote: > > > What about something like this as well > > > > F548: Unmount.png > > > > The

KDE CI: Frameworks kio kf5-qt5 XenialQt5.7 - Build # 137 - Still Unstable!

BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20XenialQt5.7/137/ Project: Frameworks kio kf5-qt5 XenialQt5.7 Date of build: Wed, 15 Nov 2017 09:43:37 + Build duration: 13 min and counting JUnit Tests Name: (root)

KDE CI: Frameworks kio kf5-qt5 FreeBSDQt5.7 - Build # 138 - Still Unstable!

BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20FreeBSDQt5.7/138/ Project: Frameworks kio kf5-qt5 FreeBSDQt5.7 Date of build: Wed, 15 Nov 2017 09:43:37 + Build duration: 10 min and counting JUnit Tests Name:

D8640: [KDirModel] Emit change for HasJobRole when jobs change

This revision was automatically updated to reflect the committed changes. Closed by commit R241:c38db223613f: [KDirModel] Emit change for HasJobRole when jobs change (authored by broulik). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8640?vs=21954=22382