D25015: Update breeze theme shadows

[Filip Fila]

filipf added a comment.


  Before:
  F7746042: Screenshot_20191107_083508.png 

  
  After:
  F7746044: Screenshot_20191107_083534.png 


REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D25015

To: niccolove, #vdg
Cc: ndavis, manueljlin, ngraham, filipf, kde-frameworks-devel, LeGast00n, GB_2, 
michaelh, bruns

D25015: Update breeze theme shadows

[Filip Fila]

filipf added a comment.


  There's definitely a difference, whereas the shadows are now rough and dark 
this is subtler.
  
  I'd suggest testing this patch with a light color scheme and then switching 
to the unpatched theme plus testing against light backgrounds to notice the 
changes.

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D25015

To: niccolove, #vdg
Cc: ndavis, manueljlin, ngraham, filipf, kde-frameworks-devel, LeGast00n, GB_2, 
michaelh, bruns

KDE CI: Frameworks » kio » kf5-qt5 FreeBSDQt5.13 - Build # 172 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20FreeBSDQt5.13/172/
 Project:
kf5-qt5 FreeBSDQt5.13
 Date of build:
Thu, 07 Nov 2019 05:39:20 +
 Build duration:
9 min 14 sec and counting
   JUnit Tests
  Name: projectroot Failed: 3 test(s), Passed: 49 test(s), Skipped: 0 test(s), Total: 52 test(s)Failed: projectroot.autotests.kiocore_kmountpointtestFailed: projectroot.autotests.kiowidgets_kdirlistertestFailed: projectroot.autotests.kiowidgets_kdirmodeltestName: projectroot.autotests Failed: 0 test(s), Passed: 6 test(s), Skipped: 0 test(s), Total: 6 test(s)Name: projectroot.src.ioslaves.trash Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot.src.kpasswdserver Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)

D25168: Use the pointing hand cursor for the single-clickable delegates

[Nathaniel Graham]

ngraham accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R304 KNewStuff

BRANCH
  use-finger-cursor-for-single-clickable-bits (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D25168

To: leinir, #knewstuff, #frameworks, ngraham
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25165: Only show DownloadItemsSheet if there's more than one download item

[Nathaniel Graham]

ngraham accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R304 KNewStuff

BRANCH
  dont-show-downloaditemssheet-if-only-one-download (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D25165

To: leinir, #knewstuff, #frameworks, ngraham
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25184: Define property X-Plasma-MainScript for Plasma/Wallpaper

[Vlad Zahorodnii]

zzag retitled this revision from "Add property definition for 
"X-Plasma-MainScript" for Plasma/Wallpaper" to "Define property 
X-Plasma-MainScript for Plasma/Wallpaper".

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D25184

To: zzag, #plasma
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25184: Add property definition for "X-Plasma-MainScript" for Plasma/Wallpaper

[Vlad Zahorodnii]

zzag created this revision.
zzag added a reviewer: Plasma.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
zzag requested review of this revision.

REVISION SUMMARY
  This commit fixes `Unknown property type for key "X-Plasma-MainScript"`
  warning that pops up when building a wallpaper plugin.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  property-def

REVISION DETAIL
  https://phabricator.kde.org/D25184

AFFECTED FILES
  src/scriptengines/qml/data/plasma-wallpaper.desktop

To: zzag, #plasma
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25173: Give some more pretty feedback in NewStuff::Page while the Engine is loading

[Nathaniel Graham]

ngraham added a comment.


  Now you don't need those PassiveNotifications anymore, because otherwise you 
wind up with duplicate messages:
  
  F7745214: loading.png 

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D25173

To: leinir, #knewstuff, #frameworks, ngraham, #vdg
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25015: Update breeze theme shadows

[Nathaniel Graham]

ngraham added a comment.


  In D25015#559367 , @ndavis wrote:
  
  > I haven't said much about this idea because I just have a hard time seeing 
the difference from the screenshots.
  
  
  I must admit having the same problem. :)

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D25015

To: niccolove, #vdg
Cc: ndavis, manueljlin, ngraham, filipf, kde-frameworks-devel, LeGast00n, GB_2, 
michaelh, bruns

D25015: Update breeze theme shadows

[Noah Davis]

ndavis added a comment.


  I haven't said much about this idea because I just have a hard time seeing 
the difference from the screenshots. It seems like a good amount of effort went 
into this, and I know from experience that adjusting shadows in Inkscape is a 
major PITA, but I can't approve something just because of that. Maybe we're 
using the wrong approach for shadows in the first place?

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D25015

To: niccolove, #vdg
Cc: ndavis, manueljlin, ngraham, filipf, kde-frameworks-devel, LeGast00n, GB_2, 
michaelh, bruns

D25015: Update breeze theme shadows

[Filip Fila]

filipf added a comment.


  I'd prefer the shadows to have equal strength all around, I couldn't get used 
to weaker shadows in the corners.

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D25015

To: niccolove, #vdg
Cc: manueljlin, ngraham, filipf, kde-frameworks-devel, LeGast00n, GB_2, 
michaelh, bruns

D25015: Update breeze theme shadows

[Niccolò Venerandi]

niccolove added a comment.


  Any update on this? I can also upload the shadow for the panel.

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D25015

To: niccolove, #vdg
Cc: manueljlin, ngraham, filipf, kde-frameworks-devel, LeGast00n, GB_2, 
michaelh, bruns

KDE CI: Frameworks » kio » kf5-qt5 FreeBSDQt5.13 - Build # 171 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20FreeBSDQt5.13/171/
 Project:
kf5-qt5 FreeBSDQt5.13
 Date of build:
Wed, 06 Nov 2019 17:30:11 +
 Build duration:
9 min 35 sec and counting
   JUnit Tests
  Name: projectroot Failed: 3 test(s), Passed: 49 test(s), Skipped: 0 test(s), Total: 52 test(s)Failed: projectroot.autotests.kiocore_kmountpointtestFailed: projectroot.autotests.kiowidgets_kdirlistertestFailed: projectroot.autotests.kiowidgets_kdirmodeltestName: projectroot.autotests Failed: 0 test(s), Passed: 6 test(s), Skipped: 0 test(s), Total: 6 test(s)Name: projectroot.src.ioslaves.trash Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot.src.kpasswdserver Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)

Re: HEADS-UP: from now do new deprecations in KF API by *_DEPRECATED_* macros only using upcoming version number

[Friedrich W. H. Kossebau]

Am Mittwoch, 6. November 2019, 16:13:28 CET schrieb David Edmundson:
> >Any chance this is about KGamma and you ran clazy with a Qt 5.14 copy
> >locally?
> It was!

Okay :) So no new holes to fix then discovered.

> >So here repeating the recommendation done by a few, which project
> >maintainers
> should pick up:
> 
> That recommendation makes sense. I'll take it to Plasma.

I recommend to then also remove the "planned-breakage-for-developers-only" 
wrapping
--- 8< ---
if (EXISTS "${CMAKE_SOURCE_DIR}/.git")
--- 8< ---
to make sure everywhere the same deprecated API visibility exists.
Also is that guard no longer needed, as this was used in pair with the 
0x06 visibility limit, to make developers bump fully into any new 
deprecations, as some seem to prefer (while I would recommend otherwise).

Cheers
Friedrich

Re: HEADS-UP: from now do new deprecations in KF API by *_DEPRECATED_* macros only using upcoming version number

[David Edmundson]

>Any chance this is about KGamma and you ran clazy with a Qt 5.14 copy locally?

It was!

>so the compiler still sees the overload and throws up.

Yeah, the whole situation makes total sense, it just wasn't something
that I had thought about when blindly running clazy.

>So here repeating the recommendation done by a few, which project maintainers
should pick up:

That recommendation makes sense. I'll take it to Plasma.

David

Re: HEADS-UP: from now do new deprecations in KF API by *_DEPRECATED_* macros only using upcoming version number

[Friedrich W. H. Kossebau]

Am Mittwoch, 6. November 2019, 14:14:41 CET schrieb David Edmundson:
> I just found a slight issue when using _DEPRECATED_SINCE.
> 
> If one of two overloaded Q_SIGNALS is deprecated, clazy will blindly
> port the signal without an overload as to the compiler only one signal
> exists. This then gives a compilation error at a distro/CI level.

Any chance this is about KGamma and you ran clazy with a Qt 5.14 copy locally?

In that case, you rather ran into this (ab)use of the visibility control:

--- 8< ---
add_definitions(-DQT_DISABLE_DEPRECATED_BEFORE=0x06)
--- 8< ---

Because QComboBox has this (not sure why "5, 15", possibly because perhaps 
post 5.14.0):
--- 8< ---
#if QT_DEPRECATED_SINCE(5, 15)
QT_DEPRECATED_VERSION_X(5, 15, "Use textActivated() instead")
void activated(const QString &);
QT_DEPRECATED_VERSION_X(5, 15, "Use textHighlighted() instead")
void highlighted(const QString &);
#endif
--- 8< ---

Given above setting, both clazy and your compiler did not see the overload 
locally with Qt 5.14.x, and all looked good. While KDE CI has Qt 5.13/5.12 
where this visibility protection does not exist, so the compiler still sees 
the overload and throws up.

I have warned about the use of 0x06 with *_DISABLE_DEPRECATED_BEFORE* , 
please read https://marc.info/?l=kde-devel=157190321318565=2

This here then should be another proof why that warning made sense.

So here repeating the recommendation done by a few, which project maintainers 
should pick up:

Set *_DISABLE_DEPRECATED_BEFORE* to the version of Qt/KF which is
the minimum required version.

To automate the latter in case of min dep version bumping, 
ECMGenerateExportHeader comes with a utility method to get the hex number from 
the min version:

--- 8< ---
ecm_export_header_format_version(${QT_MIN_VERSION} # x.y.z
HEXNUMBER_VAR qt_min_version_hexnumber # var getting 0xXXYYZZ
)

add_definitions(-DQT_DISABLE_DEPRECATED_BEFORE=${qt_min_version_hexnumber})
--- 8< ---

Cheers
Friedrich

D25164: MobileTextActionsToolBar check if controlRoot is undefined before using it

[Méven Car]

This revision was automatically updated to reflect the committed changes.
Closed by commit R242:22b66892: MobileTextActionsToolBar check if 
controlRoot is undefined before using it (authored by meven).

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25164?vs=69334=69349

REVISION DETAIL
  https://phabricator.kde.org/D25164

AFFECTED FILES
  
src/declarativeimports/plasmacomponents3/mobiletextselection/MobileTextActionsToolBar.qml

To: meven, #plasma, apol
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23384: [WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO

[Harald Sitter]

sitter added inline comments.

INLINE COMMENTS

> desktopexecparser.cpp:314
> +org::kde::KIOFuse kiofuse_iface(QStringLiteral("org.kde.KIOFuse"),
> +QStringLiteral("/org/kde/KIOFuse"),
> +QDBusConnection::sessionBus());

align arguments with first argument. same in krun.cpp.

> desktopexecparser.cpp:316
> +QDBusConnection::sessionBus());
> +QList parsedUrls;
> +QVector, QUrl>> replies;

Do we need this? Seems to me you could just append to d->url directly.

> desktopexecparser.cpp:317
> +QList parsedUrls;
> +QVector, QUrl>> replies;
>  if (!mx1.hasUrls) {

There's nothing wrong with this. But wouldn't using 
`QHash, QUrl>` make for easier to read code all in 
all since you don't have to mess with pairs?

Alternatively with a vector I'd still make a struct for the data

  struct MountRequest { QDBusPendingReply reply, QUrl url };
  QVector requests;
  ...
  requests.push_back({ mountUrl(url), url });

> desktopexecparser.cpp:319
>  if (!mx1.hasUrls) {
> -Q_FOREACH (const QUrl , d->urls)
> +for(QUrl  : d->urls) {
>  if (!url.isLocalFile() && !hasSchemeHandler(url)) {

there should be a space after for.

what happened to the constness (also in the loop below)

> kiofuseinterface.h:28
> + */
> +class KIOCORE_EXPORT KIOFuseInterface: public QDBusAbstractInterface
> +{

I don't think this should be a public/exported class. It's purely for internal 
use within KIO. It has no reason to become part of the ABI IMHO. To that end it 
also shouldn't be part of ecm_generate_headers.

I suppose this generated file could also be replaced with the actual xml and 
generated at build time instead?
Manually editing generated files is a bit meh in general.

To get the interface into both the core and widgets target I suppose you'd 
simply add it to both source lists.

Does anyone else have an opinion on this?

> krun.cpp:598
> +QList parsedUrls;
> +for (QList::Iterator it = urls.begin(); it != urls.end(); 
> ++it) {
> +QUrl url = *it;

Is there a reason you are not using a range based for loop here? `for (const 
QUrl  : urls)`

> krun.cpp:605
> +if (KIO::DesktopExecParser::isProtocolInSupportedList(url, 
> appSupportedProtocols)
> +&& url.password().isEmpty())
> +continue;

That seems like a hack for a bug in VLC and also super opinionated and also 
somewhat unrelated to fuse? If an application says it supports %u/%U and a 
given protocol, we should expect it to be able to parse a valid rfc2396 URI I 
would think.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D23384

To: feverfew, fvogt, davidedmundson, dfaure, ngraham
Cc: sitter, davidedmundson, kde-frameworks-devel, ngraham, LeGast00n, GB_2, 
michaelh, bruns

D25173: Give some more pretty feedback in NewStuff::Page while the Engine is loading

[Dan Leinir Turthra Jensen]

leinir edited the summary of this revision.
leinir edited the test plan for this revision.
leinir added reviewers: KNewStuff, Frameworks, ngraham, VDG.

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D25173

To: leinir, #knewstuff, #frameworks, ngraham, #vdg
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25173: Give some more pretty feedback in NewStuff::Page while the Engine is loading

[Dan Leinir Turthra Jensen]

leinir created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
leinir requested review of this revision.

REVISION SUMMARY
  This adds an isLoading property to KNSQuick::Engine, and that is used
  to show a busy indicator (with descriptive label) when engine is loading.

REPOSITORY
  R304 KNewStuff

BRANCH
  be-more-friendly-during-initial-load (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D25173

AFFECTED FILES
  src/qtquick/qml/Page.qml
  src/qtquick/quickengine.cpp
  src/qtquick/quickengine.h

To: leinir
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

Re: HEADS-UP: from now do new deprecations in KF API by *_DEPRECATED_* macros only using upcoming version number

[Friedrich W. H. Kossebau]

Am Mittwoch, 6. November 2019, 14:14:41 CET schrieb David Edmundson:
> I just found a slight issue when using _DEPRECATED_SINCE.
> 
> If one of two overloaded Q_SIGNALS is deprecated, clazy will blindly
> port the signal without an overload as to the compiler only one signal
> exists. This then gives a compilation error at a distro/CI level.
> 
> I don't think anything can be done, but it's worth people being wary of.

Can you give an example? I would like to look closer at it.

So far I would have assumed clazy sees the same API with the same options as 
the compiler would. I.e. any sections with visibility guards 
*_EMABLE_DEPRECATED_SINCE/*_BUILD_DEPRECATED_SINCE should work the same for 
both. So distro/CI level should not be different here.

Cheers
Friedrich

D25164: MobileTextActionsToolBar check if controlRoot is undefined before using it

[Aleix Pol Gonzalez]

apol 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/D25164

To: meven, #plasma, apol
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

Re: HEADS-UP: from now do new deprecations in KF API by *_DEPRECATED_* macros only using upcoming version number

[David Edmundson]

I just found a slight issue when using _DEPRECATED_SINCE.

If one of two overloaded Q_SIGNALS is deprecated, clazy will blindly
port the signal without an overload as to the compiler only one signal
exists. This then gives a compilation error at a distro/CI level.

I don't think anything can be done, but it's worth people being wary of.

David

D25159: Fix linking to libssh 0.9.1

[Harald Sitter]

sitter added a comment.


  D25170  for when that lands

REPOSITORY
  R320 KIO Extras

REVISION DETAIL
  https://phabricator.kde.org/D25159

To: arojas, apol, sitter, fvogt
Cc: nisavid, fvogt, sitter, asn, apol, asturmlechner, kde-frameworks-devel, 
kfm-devel, pberestov, iasensio, fprice, LeGast00n, MrPepe, fbampaloukas, 
alexde, GB_2, Codezela, feverfew, meven, michaelh, spoorun, navarromorales, 
firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov

D25170: make the libssh finder ensure the new ssh target is set

[Harald Sitter]

sitter added a comment.


  NB: depends on https://gitlab.com/libssh/libssh-mirror/merge_requests/71 
getting landed.

REPOSITORY
  R320 KIO Extras

REVISION DETAIL
  https://phabricator.kde.org/D25170

To: sitter
Cc: kde-frameworks-devel, kfm-devel, pberestov, iasensio, fprice, LeGast00n, 
MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, 
spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, 
mikesomov

KDE CI: Frameworks » extra-cmake-modules » kf5-qt5 SUSEQt5.12 - Build # 73 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/extra-cmake-modules/job/kf5-qt5%20SUSEQt5.12/73/
 Project:
kf5-qt5 SUSEQt5.12
 Date of build:
Wed, 06 Nov 2019 12:51:27 +
 Build duration:
7 min 7 sec and counting
   JUnit Tests
  Name: projectroot Failed: 2 test(s), Passed: 18 test(s), Skipped: 0 test(s), Total: 20 test(s)Failed: projectroot.tests.ECMPoQmToolsTestFailed: projectroot.tests.GenerateSipBindingsName: projectroot.tests Failed: 0 test(s), Passed: 61 test(s), Skipped: 0 test(s), Total: 61 test(s)Name: projectroot.tests.ECMAddTests Failed: 0 test(s), Passed: 2 test(s), Skipped: 0 test(s), Total: 2 test(s)
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  Cobertura Coverage Report

D25170: make the libssh finder ensure the new ssh target is set

[Harald Sitter]

sitter created this revision.
Herald added projects: Dolphin, Frameworks.
Herald added subscribers: kfm-devel, kde-frameworks-devel.
sitter requested review of this revision.

REVISION SUMMARY
  starting with 0.9.2 the libssh cmake config defines a new ssh imported
  target, use this as the new gold standard and ensure older versions
  are compatible
  
  broken ubuntu: no cmake config -> manual finder -> target injected
  0.9.0 and earlier: cmake config -> target injected
  0.9.2: cmake config -> target already defined; noop
  
  (0.9.1 is broken as it neither matches the old nor the new expectation)

TEST PLAN
  no cmake config -> target injected
  older cmake config -> target injected
  newer cmake config (with merge request) -> noop

REPOSITORY
  R320 KIO Extras

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D25170

AFFECTED FILES
  cmake/Findlibssh.cmake
  sftp/CMakeLists.txt

To: sitter
Cc: kde-frameworks-devel, kfm-devel, pberestov, iasensio, fprice, LeGast00n, 
MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, 
spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, 
mikesomov

KDE CI: Frameworks » extra-cmake-modules » kf5-qt5 SUSEQt5.13 - Build # 39 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/extra-cmake-modules/job/kf5-qt5%20SUSEQt5.13/39/
 Project:
kf5-qt5 SUSEQt5.13
 Date of build:
Wed, 06 Nov 2019 12:51:28 +
 Build duration:
7 min 7 sec and counting
   JUnit Tests
  Name: projectroot Failed: 2 test(s), Passed: 18 test(s), Skipped: 0 test(s), Total: 20 test(s)Failed: projectroot.tests.ECMPoQmToolsTestFailed: projectroot.tests.GenerateSipBindingsName: projectroot.tests Failed: 0 test(s), Passed: 61 test(s), Skipped: 0 test(s), Total: 61 test(s)Name: projectroot.tests.ECMAddTests Failed: 0 test(s), Passed: 2 test(s), Skipped: 0 test(s), Total: 2 test(s)
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  Cobertura Coverage Report

Re: Notice of intention to remove tests from KCrash and KNotifications

[Harald Sitter]

On Wed, Nov 6, 2019 at 8:07 AM Ben Cooksley  wrote:
>
> On Tue, Nov 5, 2019 at 11:50 PM Harald Sitter  wrote:
> >
> > I get where you are coming from, but honestly, none of this makes
> > pushing unreviewed commits that disable the entire test collection of
> > an entire framework any more correct. If it had only affected the one
> > test I wouldn't mind so much, but since it hasn't it only goes to
> > proof that we have reviews for a reason. In particular for repos that
> > are part of frameworks where we rely on tests to tell us if the
> > frameworks are good for the monthly release.
>
> Unfortunately reviews require responsive developers.
>
> In this instance, both Frameworks have been found to have developers
> which are not responsive (because my previous requests have been
> ignored), so sending a patch for review would have been pointless -
> because there would have been nobody to review it.
>
> Therefore bypassing review and taking any necessary action against the
> offending code (even if that does happen to be all of the tests) to
> correct the situation is the only viable option forward.

This is really not true.
We got the workaround for KCrash landed in not even half a business
day - including review.

> > There are many shades of grey between sending a "someone please fix"
> > mail to a mailing list and the nuclear option you implemented. The
> > most notable one being that you can ask someone who worked on the
> > repo, or tests recently "Hey, X, can you please disable the test on
> > windows because its impairing CI?" to which the answer is probably yes
> > because you'd not only address a very specific person but also the
> > task is doable in less than 5 minutes.
> > I understand that there's an element of cat herding in this, but
> > quality must matter for our products, and the quality of frameworks is
> > very tightly linked to autotesting and reviews because of how we
> > release them.
>
> This would require spending more time on the issue, which is something
> i'd very much rather avoid.
>
> It is expensive enough having to login to the CI builders to clear out
> these jobs when they get stuck (looking at anything that uses Akonadi
> especially here, along with these two Frameworks) and then asking the
> project lists to please fix their faulty code (which is then ignored,
> indicating that the concept of community maintained does not really
> work).
>
> Having to then lookup someone and then forward it on to them requires
> yet more time, with no guarantee that it will work - especially
> because some contributors are still hostile to any platform that is
> not FOSS and outright refuse to do anything to help those platforms.
>
> In the event that it does not work - then what would I do? Pick on the
> next person in the list (requiring yet more time)?
>
> Sorry, but that is simply too expensive, and if that is the policy
> you'd like to run with, then i'll stick to stripping projects of their
> test execution privileges outright across all platforms in the event
> they cause issues like this (which if ECM is anything to go by, is
> something people won't even notice even if the commit that puts that
> in effect is CC'ed to the project mailing list, which means that
> nobody will notice when the tests break because the CI system won't be
> running them)

I did not express myself well enough.
What I'd like you to do is talk to someone, not anyone, a very
specific someone of your choosing, to take care of the issue for you.
IOW: The sysadmins should delegate to a developer if the sysadmins
find themselves unable to accurately deal with a code problem (such as
a failing test that needs deactivating). If you tell an actually
relevant developer that's obviously better, if you don't have time to
look at the git log then that's fine too.

There are two different work items to differentiate here:

1. fixing something
2. disabling something because a fix was not made in an acceptable time frame

1 likely requires domain-knowledge of the specific framework and
probably quit a bit of time.
2 would generally only require understanding of cmake and a fairly
small time investment.

Pretty much any dev can do task 2. You just need to make someone care,
and the easiest way is to talk to a buddy directly and have them have
a look. Like if you ask me to deal with this for sysadmins I'll
definitely do. So, you have my permission to ask me to disable tests
on your behalf ;)

In short the process I would like is:

1. mail to list "yo, this framework has broken test xyz that is
severely impairing CI. plz fix"
2. if nothing happens ask someone (e.g. me) to force an action on the
repo (i.e. disable a specific test)
3. if the someone starts ghosting you you can still go in and do what
you did the other day

If you do 2 that has little overhead and a good chance of netting you
less work, not more.

HS

D25165: Only show DownloadItemsSheet if there's more than one download item

[Dan Leinir Turthra Jensen]

leinir edited the summary of this revision.
leinir added reviewers: KNewStuff, Frameworks, ngraham.

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D25165

To: leinir, #knewstuff, #frameworks, ngraham
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25168: Use the pointing hand cursor for the single-clickable delegates

[Dan Leinir Turthra Jensen]

leinir edited the summary of this revision.
leinir added reviewers: KNewStuff, Frameworks, ngraham.

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D25168

To: leinir, #knewstuff, #frameworks, ngraham
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25168: Use the pointing hand cursor for the single-clickable delegates

[Dan Leinir Turthra Jensen]

leinir created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
leinir requested review of this revision.

REPOSITORY
  R304 KNewStuff

BRANCH
  use-finger-cursor-for-single-clickable-bits (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D25168

AFFECTED FILES
  src/qtquick/qml/private/entrygriddelegates/BigPreviewDelegate.qml
  src/qtquick/qml/private/entrygriddelegates/ThumbDelegate.qml
  src/qtquick/qml/private/entrygriddelegates/TileDelegate.qml

To: leinir
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25165: Only show DownloadItemsSheet if there's more than one download item

[Dan Leinir Turthra Jensen]

leinir created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
leinir requested review of this revision.

REVISION SUMMARY
  BUG:413437

TEST PLAN
  With patch: Shows "Install..." for all items (with other than one user 
download)
  Without patch: Shows "Install..." only for items with more than one download 
item

REPOSITORY
  R304 KNewStuff

BRANCH
  dont-show-downloaditemssheet-if-only-one-download (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D25165

AFFECTED FILES
  src/qtquick/qml/private/entrygriddelegates/BigPreviewDelegate.qml
  src/qtquick/qml/private/entrygriddelegates/ThumbDelegate.qml
  src/qtquick/qml/private/entrygriddelegates/TileDelegate.qml

To: leinir
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25164: MobileTextActionsToolBar check if controlRoot is undefined before using it

[Méven Car]

meven created this revision.
meven added a reviewer: Plasma.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
meven requested review of this revision.

REVISION SUMMARY
  Prevents warning such as
  
  
qml/org/kde/plasma/components.3/mobiletextselection/MobileTextActionsToolBar.qml:62:
 TypeError: Cannot read property 'selectedText' of null
  
qml/org/kde/plasma/components.3/mobiletextselection/MobileTextActionsToolBar.qml:70:
 TypeError: Cannot read property 'selectedText' of null
  
qml/org/kde/plasma/components.3/mobiletextselection/MobileTextActionsToolBar.qml:78:
 TypeError: Cannot read property 'canPaste' of null

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D25164

AFFECTED FILES
  
src/declarativeimports/plasmacomponents3/mobiletextselection/MobileTextActionsToolBar.qml

To: meven, #plasma
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24962: [DeleteJob] Use a separate worker thread to run actual IO operation

[Méven Car]

meven added inline comments.

INLINE COMMENTS

> deletejob.cpp:130
> +/// Callback of worker rmfile
> +void rmFileResult(bool result, bool isLink);
> +/// Callback of worker rmdir

Now we are at it, isLink could become m_curentURLIsLink, but isLink being a 
bool, it is much cheaper to move around threads, so we don't have perf 
incentive to do it as we had with a QString.

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D24962

REVISION DETAIL
  https://phabricator.kde.org/D24962

To: meven, dfaure, #frameworks
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24962: [DeleteJob] Use a separate worker thread to run actual IO operation

[Méven Car]

meven updated this revision to Diff 69332.
meven marked 4 inline comments as done.
meven added a comment.


  Use m_current instead of passing const QUrl  around

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24962?vs=68946=69332

BRANCH
  arcpatch-D24962

REVISION DETAIL
  https://phabricator.kde.org/D24962

AFFECTED FILES
  src/core/deletejob.cpp

To: meven, dfaure, #frameworks
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25159: Fix linking to libssh 0.9.1

[Andreas Schneider]

asn added a comment.


  I've created https://gitlab.com/libssh/libssh-mirror/merge_requests/71
  
  This means the right code would be then.
  
if (TARGET ssh) # libssh 0.9+
target_link_libraries(kio_sftp ssh)
endif()

REPOSITORY
  R320 KIO Extras

REVISION DETAIL
  https://phabricator.kde.org/D25159

To: arojas, apol, sitter, fvogt
Cc: fvogt, sitter, asn, apol, asturmlechner, kde-frameworks-devel, kfm-devel, 
pberestov, iasensio, fprice, LeGast00n, MrPepe, fbampaloukas, alexde, GB_2, 
Codezela, feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, mikesomov

D24962: [DeleteJob] Use a separate worker thread to run actual IO operation

[Méven Car]

meven edited the summary of this revision.

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D24962

REVISION DETAIL
  https://phabricator.kde.org/D24962

To: meven, dfaure, #frameworks
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns