D11880: Add firewall-config and firewall-applet icons

2018-09-21 Thread Noah Davis
ndavis updated this revision to Diff 42119. ndavis added a comment. Change to bruns/state-* hybrid style REPOSITORY R266 Breeze Icons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D11880?vs=41973&id=42119 BRANCH firewalld-icons (branched from master) REVISION DETAIL https://

D13627: [KSharedDataCache] Assume lock before flush changes

2018-09-21 Thread Anthony Fieroni
anthonyfieroni added a comment. It looks can happen https://www.reddit.com/r/kde/comments/9hovrv/ive_been_having_this_randomly_icons_appears_to_be/ But i'm not sure that patch can handle it. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D13627 To: anthonyfier

D11880: Add firewall-config and firewall-applet icons

2018-09-21 Thread Noah Davis
ndavis added a comment. In D11880#329995 , @bruns wrote: > If you use "{xxx, size=full}", you can avoid the scaling (preferable for small inline images). Fixed. Thanks! REPOSITORY R266 Breeze Icons BRANCH firewalld-icons (branched f

D11880: Add firewall-config and firewall-applet icons

2018-09-21 Thread Stefan Brüns
bruns added a comment. In D11880#329994 , @ndavis wrote: > 16px > F6278474: Screenshot_20180921_230725.png > > 22px > F6278475: Screenshot_20180921_230711.png

D11880: Add firewall-config and firewall-applet icons

2018-09-21 Thread Noah Davis
ndavis added a comment. Here are some new styles based on suggestions and @bruns's ideas. I've also tried to make one of the new styles looks similar to the status/*/state-* icons. 16px F6278474: Screenshot_20180921_230725.png 22px F6278475

D15685: Fix Android builds using cmake 3.12.1

2018-09-21 Thread Aleix Pol Gonzalez
apol created this revision. apol added reviewers: Frameworks, vkrause. Herald added projects: Frameworks, Build System. Herald added subscribers: kde-buildsystem, kde-frameworks-devel. apol requested review of this revision. REVISION SUMMARY Only extract the the stl shared object without the res

D11880: Add firewall-config and firewall-applet icons

2018-09-21 Thread Noah Davis
ndavis added a comment. I think I'm confused about the process here. Are we using @bruns's icons now? In D11880#329896 , @bruns wrote: > F6277682: out.png > > How about these - error, "trusted zone",

D15091: Compile python bindings with the same sip flags used by PyQt

2018-09-21 Thread Antonio Rojas
arojas updated this revision to Diff 42116. arojas added a comment. Rebase REPOSITORY R240 Extra CMake Modules CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15091?vs=40570&id=42116 REVISION DETAIL https://phabricator.kde.org/D15091 AFFECTED FILES find-modules/FindPythonModu

D15645: [WIP] Add scheme selection menu with a "System" entry.

2018-09-21 Thread Amish Naidu
amhndu marked 2 inline comments as done. REPOSITORY R265 KConfigWidgets REVISION DETAIL https://phabricator.kde.org/D15645 To: amhndu, #frameworks Cc: broulik, kde-frameworks-devel, michaelh, ngraham, bruns

D15645: [WIP] Add scheme selection menu with a "System" entry.

2018-09-21 Thread Amish Naidu
amhndu updated this revision to Diff 42108. amhndu added a comment. Move duplicate code from the two createSelectionMenu methods into one generic createSelectionMenu in KColorSchemeMangerPrivate REPOSITORY R265 KConfigWidgets CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15645?

D11880: Add firewall-config and firewall-applet icons

2018-09-21 Thread Sven Mauch
svenmauch added a comment. Yup! +1 REPOSITORY R266 Breeze Icons BRANCH firewalld-icons (branched from master) REVISION DETAIL https://phabricator.kde.org/D11880 To: ndavis, #vdg, #breeze, ngraham Cc: bruns, abetts, alex-l, svenmauch, kde-frameworks-devel, ngraham, michaelh

D11880: Add firewall-config and firewall-applet icons

2018-09-21 Thread Andres Betts
abetts added a comment. I solemnly approve! +1 REPOSITORY R266 Breeze Icons BRANCH firewalld-icons (branched from master) REVISION DETAIL https://phabricator.kde.org/D11880 To: ndavis, #vdg, #breeze, ngraham Cc: bruns, abetts, alex-l, svenmauch, kde-frameworks-devel, ngraham, michaelh

D11880: Add firewall-config and firewall-applet icons

2018-09-21 Thread Nathaniel Graham
ngraham added a comment. Now that's what I'm talkin' about. Those are awesome in my book. Other #VDG folks, thoughts? REPOSITORY R266 Breeze Icons BRANCH firewalld-icons (branched from master) REVISION DETAIL https://phabricator.kde.org/D11880 T

D11880: Add firewall-config and firewall-applet icons

2018-09-21 Thread Stefan Brüns
bruns added a comment. Alternate error version: F6277862: out.png F6277861: icons_apps_22_firewall-applet-error.svg REPOSITORY R266 Breeze Icons BRANCH firewalld-icons (branched from master) REVISION DE

D11880: Add firewall-config and firewall-applet icons

2018-09-21 Thread Stefan Brüns
bruns added a comment. F6277841: out.png F6277844: icons_apps_22_firewall-applet-open.svg F6277843: icons_apps_22_firewall-applet.svg F6277842: icons_apps_22_fir

D15643: Android: Allow passing a relative path as the apk dir

2018-09-21 Thread Johnny Jazeix
jjazeix added a comment. Unfortunately, this does not work. The tmpAndroid folder is created in the build folder after running cmake, so it's not available before: Cannot find tmpAndroid//AndroidManifest.xml according to ANDROID_APK_DIR. tmpAndroid/ GCompris Call Stack (most re

D11880: Add firewall-config and firewall-applet icons

2018-09-21 Thread Nathaniel Graham
ngraham added a comment. I like your depiction of the logical consequences of the firewall being depicted as a wall, @bruns. That seems great. I think a further exploration is warranted: `Error:` wall is damaged or compromised; maybe holes in it or a pile of brick rubble! Red color is a

D11880: Add firewall-config and firewall-applet icons

2018-09-21 Thread Andres Betts
abetts added a comment. Seems good to me. Guys? REPOSITORY R266 Breeze Icons BRANCH firewalld-icons (branched from master) REVISION DETAIL https://phabricator.kde.org/D11880 To: ndavis, #vdg, #breeze, ngraham Cc: bruns, abetts, alex-l, svenmauch, kde-frameworks-devel, ngraham, michael

D11880: Add firewall-config and firewall-applet icons

2018-09-21 Thread Stefan Brüns
bruns added a comment. F6277682: out.png How about these - error, "trusted zone", panic, "public/untrusted" F6277688: icons_apps_22_firewall-applet.svg F6277687: icons_apps_22_firewall-applet-open.svg

D15644: Provide option to hide menu bar for Ksysguard - Bug 395349

2018-09-21 Thread Nathaniel Graham
ngraham added inline comments. INLINE COMMENTS > ksysguard.cpp:149 > + > + // setup 'Settings' menu > + KToggleAction* showMenuBar = KStandardAction::showMenubar(nullptr, > nullptr, actionCollection()); Minor nitpick: "setup" is a noun; it should be "set up" so that there's a verb in the

D15644: Provide option to hide menu bar for Ksysguard - Bug 395349

2018-09-21 Thread Nathaniel Graham
ngraham added a comment. Thanks! Please also remove ` - Bug 395349` from the title and replace it with `BUG: 395349` in the Summary section. --- Now the patch looks better, applies cleanly, and works in my testing. However--and this is a complaint with other KDE software as well--if

D15353: Use _NET_WM_WINDOW_TYPE_COMBO instead of _NET_WM_WINDOW_TYPE_COMBOBOX

2018-09-21 Thread Vlad Zagorodniy
zzag added a comment. In D15353#329431 , @romangg wrote: > Whoops, yea. My fault. I think it's fine to replace the name, since Qt/GTK use the correct one and normally apps use these libs instead of setting such properties directly. Can

D15644: Provide option to hide menu bar for Ksysguard - Bug 395349

2018-09-21 Thread Luca Sartorelli
lsartorelli updated this revision to Diff 42101. lsartorelli retitled this revision from "Bug 395349" to "Provide option to hide menu bar for Ksysguard - Bug 395349". lsartorelli edited the summary of this revision. lsartorelli added a comment. Thank you, for all the feedback. Yes, I have

D15644: Bug 395349

2018-09-21 Thread Andrew Crouthamel
acrouthamel added a comment. Yeah I think that is a mistake. D42047 which precedes the current diff, makes much more sense. REVISION DETAIL https://phabricator.kde.org/D15644 To: lsartorelli, ngraham, #plasma, #frameworks Cc: acrouthamel, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, les

D15644: Bug 395349

2018-09-21 Thread Nathaniel Graham
ngraham requested changes to this revision. ngraham added reviewers: Plasma, Frameworks. ngraham added a comment. This revision now requires changes to proceed. Thank you for the patch! I see that this is your first KDE contribution, how exciting! This needs some work, so please don't get disc

D15646: [sftp] Remove unused variable type

2018-09-21 Thread Andreas Schneider
asn abandoned this revision. REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D15646 To: asn, sitter, ngraham Cc: kde-frameworks-devel, kfm-devel, feverfew, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp

D15645: [WIP] Add scheme selection menu with a "System" entry.

2018-09-21 Thread Amish Naidu
amhndu added a comment. Should I also add the other overloads like `createSchemeSelectionMenu` does ? INLINE COMMENTS > broulik wrote in kcolorschememanager.cpp:214-228 > All of this is duplicated from`createSchemeSelectionMenu`, it should be split > into a separate method so it can be reuse

D15646: [sftp] Remove unused variable type

2018-09-21 Thread Andreas Schneider
asn added a comment. Could you please talk to me the next time before you're going to push patches from a branch I'm working on? Communication helps to avoid double work. REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D15646 To: asn, sitter, ngraham Cc: kde-

D11880: Add firewall-config and firewall-applet icons

2018-09-21 Thread Andres Betts
abetts added a comment. If I understand right, there are three states of firewall security that you can be in. Maybe we could use the traffic lights metaphor and have green for low, yellow for medium and red for high? REPOSITORY R266 Breeze Icons BRANCH firewalld-icons (branched from ma

D15646: [sftp] Remove unused variable type

2018-09-21 Thread Harald Sitter
sitter added a comment. already landed? REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D15646 To: asn, sitter, ngraham Cc: kde-frameworks-devel, kfm-devel, feverfew, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp

D15646: [sftp] Remove unused variable type

2018-09-21 Thread Andreas Schneider
asn created this revision. asn added reviewers: sitter, ngraham. Herald added projects: Dolphin, Frameworks. Herald added subscribers: kfm-devel, kde-frameworks-devel. asn requested review of this revision. REVISION SUMMARY Signed-off-by: Andreas Schneider REPOSITORY R320 KIO Extras REVISIO

D15645: [WIP] Add scheme selection menu with a "System" entry.

2018-09-21 Thread Kai Uwe Broulik
broulik added inline comments. INLINE COMMENTS > kcolorschememanager.cpp:203 > +QAction *const resetAction = new > QAction(index.data(Qt::DecorationRole).value(), > + QStringLiteral("System > (%1)").arg(systemScheme), > +

D15645: [WIP] Add scheme selection menu with a "System" entry.

2018-09-21 Thread Amish Naidu
amhndu added a reviewer: Frameworks. REPOSITORY R265 KConfigWidgets REVISION DETAIL https://phabricator.kde.org/D15645 To: amhndu, #frameworks Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15645: [WIP] Add scheme selection menu with a "System" entry.

2018-09-21 Thread Amish Naidu
amhndu created this revision. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. amhndu requested review of this revision. REVISION SUMMARY The scheme selection menu does not offer any hint for the current system scheme. This also makes it hard to "reset" back

D15643: Android: Allow passing a relative path as the apk dir

2018-09-21 Thread Johnny Jazeix
jjazeix added a comment. Seems good to me at first sight, I'll try it as soon as possible REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D15643 To: apol, #frameworks, #gcompris Cc: jjazeix, kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns

D6513: Add support for Attica tags support

2018-09-21 Thread Dan Leinir Turthra Jensen
leinir added a comment. For ease of re-reviewing, the relevant bits of the patch (the bits that didn't already get the tickmark) can be seen on their own like so: https://phabricator.kde.org/D6513?vs=41133&id=41886&whitespace=ignore-most#toc (thank you very much differential for being a

D15353: Use _NET_WM_WINDOW_TYPE_COMBO instead of _NET_WM_WINDOW_TYPE_COMBOBOX

2018-09-21 Thread Roman Gilg
romangg added a comment. Whoops, yea. My fault. I think it's fine to replace the name, since Qt/GTK use the correct one and normally apps use these libs instead of setting such properties directly. REPOSITORY R278 KWindowSystem REVISION DETAIL https://phabricator.kde.org/D15353 To: zz

D11880: Add firewall-config and firewall-applet icons

2018-09-21 Thread Alessandro Longo
alex-l added a comment. Very cool icons! But in the panic one the padlock is too small :-/ what a about a checkmark (✅) on the wall instead? REPOSITORY R266 Breeze Icons BRANCH firewalld-icons (branched from master) REVISION DETAIL https://phabricator.kde.org/D11880 To: ndavis, #vdg,

D15353: Use _NET_WM_WINDOW_TYPE_COMBO instead of _NET_WM_WINDOW_TYPE_COMBOBOX

2018-09-21 Thread Vlad Zagorodniy
zzag added a comment. In D15353#329424 , @romangg wrote: > ... to KWindowSystem as well? Isn't it KWindowSystem repo? (maybe, I don't understand your question) REPOSITORY R278 KWindowSystem REVISION DETAIL https://phabricator.kde.org

D15353: Use _NET_WM_WINDOW_TYPE_COMBO instead of _NET_WM_WINDOW_TYPE_COMBOBOX

2018-09-21 Thread Roman Gilg
romangg added a comment. In D15353#323028 , @zzag wrote: > In D15353#322991 , @aacid wrote: > > > I'm not a KWindowSystem developer so take this with a grain of salt, but maybe it makes sense to stil

D15353: Use _NET_WM_WINDOW_TYPE_COMBO instead of _NET_WM_WINDOW_TYPE_COMBOBOX

2018-09-21 Thread Vlad Zagorodniy
zzag added a comment. Ping. REPOSITORY R278 KWindowSystem REVISION DETAIL https://phabricator.kde.org/D15353 To: zzag, #kwin Cc: aacid, kde-frameworks-devel, michaelh, ngraham, bruns

D15643: Android: Allow passing a relative path as the apk dir

2018-09-21 Thread Aleix Pol Gonzalez
apol created this revision. apol added reviewers: Frameworks, GCompris. Herald added projects: Frameworks, Build System. Herald added subscribers: kde-buildsystem, kde-frameworks-devel. apol requested review of this revision. REVISION SUMMARY This way the project can generate automatically its o

D15635: Use String to store UDS_USER and UDS_GROUP of String type.

2018-09-21 Thread Jaime Torres Amate
jtamate updated this revision to Diff 42044. jtamate retitled this revision from "Use String to store UDS_USER and UDS_GROUP of String type, use fastInsert." to "Use String to store UDS_USER and UDS_GROUP of String type.". jtamate edited the summary of this revision. jtamate edited the test plan

D15638: force-finish canberra notifications on close()

2018-09-21 Thread Jaime Torres Amate
jtamate accepted this revision. jtamate added a comment. This revision is now accepted and ready to land. Forget what I just wrote. I've seen that I was missing 15 lines in between then in the phabricator diff view. :-( Looks good to me. REPOSITORY R289 KNotifications BRANCH master RE

D15638: force-finish canberra notifications on close()

2018-09-21 Thread Jaime Torres Amate
jtamate added a comment. What would happen if finishNotification is called twice, In line 161 and then in line 192? My guess is that finished signal will be called twice with the same notification, and therefore KNotificationManager::notifyPluginFinished will be called twice. I don't know

D15635: Use String to store UDS_USER and UDS_GROUP of String type, use fastInsert.

2018-09-21 Thread Kai Uwe Broulik
broulik added a comment. I also would prefer to do the `fastInsert` change separately as to not clutter the diff REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D15635 To: jtamate, dfaure, #baloo, #frameworks Cc: broulik, kde-frameworks-devel, ashaposhnikov, michaelh,

D15635: Use String to store UDS_USER and UDS_GROUP of String type, use fastInsert.

2018-09-21 Thread David Faure
dfaure added a comment. The question of the caching of the KUser usage remains, though. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D15635 To: jtamate, dfaure, #baloo, #frameworks Cc: broulik, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, ngrah

D15638: force-finish canberra notifications on close()

2018-09-21 Thread Harald Sitter
sitter created this revision. sitter added a reviewer: broulik. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. sitter requested review of this revision. REVISION SUMMARY `KNotification::close()` causes the manager to close the plugin for the notification a

D15635: Use String to store UDS_USER and UDS_GROUP of String type, use fastInsert.

2018-09-21 Thread Jaime Torres Amate
jtamate updated this revision to Diff 42033. jtamate retitled this revision from "Use String to store UDS_USER and UDS_GROUP de tipo String" to "Use String to store UDS_USER and UDS_GROUP of String type, use fastInsert.". jtamate edited the summary of this revision. jtamate added a comment. W

D15623: Finish the notification before removing it from the hash

2018-09-21 Thread Harald Sitter
sitter added a comment. Don't fret, it was a good idea :) REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D15623 To: jtamate, #frameworks, sitter Cc: anthonyfieroni, kde-frameworks-devel, michaelh, ngraham, bruns

D15623: Finish the notification before removing it from the hash

2018-09-21 Thread Jaime Torres Amate
jtamate abandoned this revision. jtamate added a comment. I didn't realize the notifications are not owned by m_notifications :-(. Just looking at the backtrace of the crash, it was the only suspicious thing. REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D1

D15635: Use String to store UDS_USER and UDS_GROUP de tipo String

2018-09-21 Thread Kai Uwe Broulik
broulik added a comment. Fine in general, `file` KIO caches the username and groupname to avoud `KUser` having to query it every time. It also uses the `uid` as string if `loginName` is empty (if that can ever happen..?) REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/

D11880: Add firewall-config and firewall-applet icons

2018-09-21 Thread Sven Mauch
svenmauch added a comment. I like the regular icon, looks great! Though I'm unsure about the panic icon. It communicates "all is well" to me instead of "all network traffic blocked". Maybe there is a better way to achieve this. An orange icon with a spread out hand in front of it? Just a

D15635: Use String to store UDS_USER and UDS_GROUP de tipo String

2018-09-21 Thread Jaime Torres Amate
jtamate created this revision. jtamate added reviewers: dfaure, Baloo, Frameworks. Herald added projects: Frameworks, Baloo. Herald added a subscriber: kde-frameworks-devel. jtamate requested review of this revision. REVISION SUMMARY First crash I get after enabling Dr. Konqi for slaves. UDS_U

D15623: Finish the notification before removing it from the hash

2018-09-21 Thread Harald Sitter
sitter added a comment. I am not sure I understand this. (admittedly I hadn't had enough coffee yet though) NotifyByAudio doesn't own the `KNotification *`, it gets it form the outside via `::notify()`. Why would removing the entry from the internal "cache" hash m_notifications cause it