KDE CI: Frameworks » kdelibs4support » kf5-qt5 FreeBSDQt5.14 - Build # 3 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kdelibs4support/job/kf5-qt5%20FreeBSDQt5.14/3/
 Project:
kf5-qt5 FreeBSDQt5.14
 Date of build:
Thu, 09 Apr 2020 04:20:18 +
 Build duration:
30 min and counting
   JUnit Tests
  Name: projectroot Failed: 2 test(s), Passed: 37 test(s), Skipped: 0 test(s), Total: 39 test(s)Failed: projectroot.autotests.kmimetypetestFailed: projectroot.autotests.kstandarddirstest

D28682: export done signal in filecontentindexer

[Stefan Brüns]

bruns added a comment.


  In D28682#644430 , @astippich 
wrote:
  
  > In D28682#644414 , @bruns wrote:
  >
  > > In general fine for me.
  > >
  > > How will Elisa deal with the indexer maybe crashing inbetween? Do we also 
need a signal for a batch start?
  >
  >
  > From my (limited) understanding, the finishedIndexingFile will no be 
emitted when an indexers crashes, does it? Then I think it is fine without an 
additional signal as long as the done() signal is still emitted as it is 
possible to reset the applications' internal list. Or will it retry some files 
of the current batch?
  
  
  It will not be emitted for the file it crashed on, but for the earlier files. 
So you may have the following for the files 'a, b, c, d':
  
  - S:a, F:a
  - S:b, F:b
  - S:c 
  - S:a, F:a
  - S:b, F:b
  - done
  - S:c, 
  - S:d, F:d
  - done
  
  So when you only listen to `finished` and `done`, you should keep the files 
in a `set`, but after the `done` the earlier files should be committed.

REPOSITORY
  R293 Baloo

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

To: astippich, #baloo, bruns
Cc: mgallien, kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n, cblack, 
fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D28682: export done signal in filecontentindexer

[Alexander Stippich]

astippich added a comment.


  > Quoted Text
  > 
  >> ! In D28682#644412 , @mgallien 
wrote:
  > 
  > This is the reason why it is so slow in Elisa?
  
  Do you mean slow for picking up changes in files? Then yes. This happens when 
a track is modified when Elisa is running and listens for finishedIndexingFile 
signal. It then immediately queries baloo database, which still has the old 
values.
  Only after the done signal the transaction to baloo's database is complete.
  
  In D28682#644414 , @bruns wrote:
  
  > In general fine for me.
  >
  > How will Elisa deal with the indexer maybe crashing inbetween? Do we also 
need a signal for a batch start?
  
  
  From my (limited) understanding, the finishedIndexingFile will no be emitted 
when an indexers crashes, does it? Then I think it is fine without an 
additional signal as long as the done() signal is still emitted as it is 
possible to reset the applications' internal list. Or will it retry some files 
of the current batch?

REPOSITORY
  R293 Baloo

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

To: astippich, #baloo, bruns
Cc: mgallien, kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n, cblack, 
fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D28679: [KPropertiesDialog] Disable changing dir icon on samba shares

[Ahmad Samir]

ahmadsamir added a comment.


  OK, thanks for the pointers.

REPOSITORY
  R241 KIO

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

To: ahmadsamir, #frameworks, dfaure, sitter
Cc: broulik, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D28682: export done signal in filecontentindexer

[Stefan Brüns]

bruns added a comment.


  In general fine for me.
  
  How will Elisa deal with the indexer maybe crashing inbetween? Do we also 
need a signal for a batch start?

REPOSITORY
  R293 Baloo

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

To: astippich, #baloo, bruns
Cc: mgallien, kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n, cblack, 
fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D28682: export done signal in filecontentindexer

[Matthieu Gallien]

mgallien added a comment.


  This is the reason why it is so slow in Elisa?

REPOSITORY
  R293 Baloo

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

To: astippich, #baloo, bruns
Cc: mgallien, kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n, cblack, 
fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D28669: make CopyJob non-recursive

[David Faure]

dfaure requested changes to this revision.
dfaure added inline comments.

INLINE COMMENTS

> copyjob.cpp:809
>  ++m_currentStatSrc;
> -statCurrentSrc();
>  }

Why did you remove this call?

Your patch has no context (please use `arc` to upload patches to phabricator), 
but I can see here that this is in the CopyJobPrivate::skipSrc method. Did you 
test skipping files? This isn't mentioned in your test plan.

Does the unittest jobtest still pass?

> anthonyfieroni wrote in copyjob.cpp:814
> It'll not help, message queue will be blocked and you kill the from other 
> thread, which is not what we want.

Which threads? There are no threads involved here.

There is no need to handling killing here. It wasn't any different in the orig 
code with the recursion.
As soon as we find actual work to do, we'll go and launch a subjob, at which 
point we go back to the event loop and can handle being killed.

> copyjob.cpp:849
>  files.append(info);   // Files and any symlinks
>  statNextSrc(); // we could use a loop instead of a recursive 
> call :)
> +continue;

This still recurses. How does this get rid of recursion? I don't get it.

> meven wrote in copyjob.cpp:892
> I guess we can update this comment

Why? it still recurses.

> copyjob.cpp:915
>  m_bURLDirty = true;
> -} else {
> +}
> +

This makes no sense to me. We are finished when the previous phase is actually 
finished.

REPOSITORY
  R241 KIO

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

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

D28406: Fix sonnet autodetect test failure

[David Faure]

dfaure added a comment.


  @waqar ping? What do you think about my suggestion? If you agree, can you 
update the patch?

REPOSITORY
  R246 Sonnet

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

To: waqar, dfaure
Cc: dfaure, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D28682: export done signal in filecontentindexer

[Alexander Stippich]

astippich created this revision.
astippich added reviewers: Baloo, bruns.
Herald added projects: Frameworks, Baloo.
Herald added a subscriber: kde-frameworks-devel.
astippich requested review of this revision.

REVISION SUMMARY
  Applications can subsribe for new files being indexed by baloo
  via the dbus interface of fileindexer. However, currently only
  the start and finishedIndexing signals are exported.
  The latter does only indicate that the file has been indexed,
  but not yet committed to the database.  
  Export the done signal so that it is possible to know
  when the transaction has been committed and thus is safe to
  query the database for metadata.

REPOSITORY
  R293 Baloo

BRANCH
  exportSignal

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

AFFECTED FILES
  src/file/filecontentindexer.h

To: astippich, #baloo, bruns
Cc: kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n, cblack, 
fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D28406: Fix sonnet autodetect test failure

[David Faure]

dfaure requested changes to this revision.
This revision now requires changes to proceed.

REPOSITORY
  R246 Sonnet

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

To: waqar, dfaure
Cc: dfaure, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D28406: Fix sonnet autodetect test failure

[David Faure]

dfaure added a reviewer: dfaure.

REPOSITORY
  R246 Sonnet

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

To: waqar, dfaure
Cc: dfaure, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

[Kevin Ottens]

ervin updated this revision to Diff 79656.
ervin marked 6 inline comments as done.
ervin added a comment.


  Addresses David's comments

REPOSITORY
  R265 KConfigWidgets

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27540?vs=78703&id=79656

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

AFFECTED FILES
  src/CMakeLists.txt
  src/kconfigdialogmanager.cpp
  src/kconfigdialogmanager.h
  src/kconfigdialogmanager_p.h
  src/settingsstatusindicator.cpp
  src/settingsstatusindicator_p.h

To: ervin, ngraham, davidedmundson, meven, crossi, bport, #vdg, ndavis, broulik
Cc: alexde, ndavis, iasensio, davidre, kde-frameworks-devel, LeGast00n, cblack, 
GB_2, michaelh, ngraham, bruns

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

[Kevin Ottens]

ervin marked 6 inline comments as done.
ervin added inline comments.

INLINE COMMENTS

> davidedmundson wrote in kconfigdialogmanager.cpp:609
> Why not item->readDefault()?

Wouldn't do the same thing at all. readDefault() takes a KConfig object and 
updates the default value stored in the item with what it found in there.

Yes, I know... the item API is weird...

> davidedmundson wrote in kconfigdialogmanager.cpp:625
> won't it do it itself when the property changes?

Good point, was indeed unnecessary now, I removed the line.

> davidedmundson wrote in settingsstatusindicator.cpp:75
> > This is equivalent to calling showFullScreen(), showMaximized(), or 
> > setVisible(true), depending on the platform's default behavior for the 
> > window flags.
> 
> For X and wayland it's setVisible(true)
> 
> but we shouldn't count on it.

I don't think it matters for widgets which have a parent and not the Qt::Window 
window flag, but OK, switching to setVisible() instead of show/hide.

> davidedmundson wrote in settingsstatusindicator.cpp:175
> unused?

I'm not sure how you end up to this conclusion, it's written two below if we 
are at the window edge, and it's used in the move call at the end.

> davidedmundson wrote in settingsstatusindicator.cpp:184
> Can we be sure the tracked widget always has a parent widget?
> 
> If someone doesn't use layouts a widget might not have a parent.

Well that'd mean the tracked widget is a window... it's pretty much an 
impossibility IMO.

> davidedmundson wrote in settingsstatusindicator.cpp:192
> that's not true for the RTL case where the widget is expected to resize.
> 
> It would be   w->pos().x() + w->width() - widgetExpectedWidth(w)

Either I misunderstood what you meant or you got the math wrong on that one.

What you're proposing (or what I understood you're proposing) breaks the "RTL + 
widget at edge" case in my tests. The line I wrote is working perfectly fine 
for my tests with desktoppath and qtquicksettings both in LTR and RTL modes.

REPOSITORY
  R265 KConfigWidgets

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

To: ervin, ngraham, davidedmundson, meven, crossi, bport, #vdg, ndavis, broulik
Cc: alexde, ndavis, iasensio, davidre, kde-frameworks-devel, LeGast00n, cblack, 
GB_2, michaelh, ngraham, bruns

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

[Kevin Ottens]

ervin added a comment.


  In D27540#638985 , @ndavis wrote:
  
  > Somehow I missed the notification that this was updated.
  >
  > Thanks for the horizontal alignment. Could you also add a left margin to 
the column of reset buttons? It should be the same as the spacing between the 
labels and the controls, which is equivalent to `Kirigami.Units.smallSpacing`, 
IIRC.
  
  
  Actually was already the case, forgot to update the screenshot.

REPOSITORY
  R265 KConfigWidgets

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

To: ervin, ngraham, davidedmundson, meven, crossi, bport, #vdg, ndavis, broulik
Cc: alexde, ndavis, iasensio, davidre, kde-frameworks-devel, LeGast00n, cblack, 
GB_2, michaelh, ngraham, bruns

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

[Kevin Ottens]

ervin edited the test plan for this revision.

REPOSITORY
  R265 KConfigWidgets

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

To: ervin, ngraham, davidedmundson, meven, crossi, bport, #vdg, ndavis, broulik
Cc: alexde, ndavis, iasensio, davidre, kde-frameworks-devel, LeGast00n, cblack, 
GB_2, michaelh, ngraham, bruns

Re: Problems in KWayland causes by API and ABI compatibility promises

[Kevin Ottens]

Hello,

On Wednesday, 1 April 2020 14:04:10 CEST David Edmundson wrote:
> Here is a list of active uses of the KWayland::Client API.
> 
> frameworks
> plasma-framework (for window positioning)
> 
> apps:
> spectacle (for window positioning)
> kdeconnect-kde  (for fake input)
> yakuake (for window positioning)
> 
> extragear
> latte-dock (for window positioning, custom shadow (which could be
> ported already) and windowmanagement)

The part of the list above makes me wonder, shouldn't the window positioning 
and windowmanager features be on KWindowSystem?

I got no idea regarding kdeconnect-kde and the fake input...
 
> workspace:
> kwin unit tests
> libkscreen
> breeze (till Qt5.15)
> oxyen (till Qt5.15)
> xdg-desktop-portal
> kinfocenter
> plasma-workspace
> plasma-nano
> kscreenlocker
> powerdevil
> kwayland-integration (the backend for kwindowsystem)
> plasma-phone-components
> plasma-integration

I think the above is less of an issue, right? Because workspace would have a 
copy of KWayland which could be shared with whatever stability constraints 
needed.

Hope that helps. :-)

Regards.
-- 
Kevin Ottens, http://ervin.ipsquad.net

enioka Haute Couture - proud patron of KDE, https://hc.enioka.com/en

signature.asc
Description: This is a digitally signed message part.

Re: Tips for testing and debugging KWindowSystem

[Aleix Pol]

Hi Victor,
We usually recommend installing the packages to test into a separate
prefix so you can only test whatever you want against it, so that you
don't break your own system.

Now for more details I guess we'd need to know what you're trying to improve.

Aleix

On Tue, Apr 7, 2020 at 11:49 PM Victor Dodon  wrote:
>
> Hi all,
>
> Can someone give me tips on how to test KWindowSystem while developing? I 
> have a dual 4K monitor setup and I have found a bug that seems to reside in 
> KWindowSystem. I would like to try to fix it and then submit a patch, but I 
> would like to know what's the best way to test and especially to debug this 
> component. Especially, is it safe to install alongside system packages (I'm 
> running Arch linux)? How can I debug it (gdb, qDebug())?
>
> Kind regards,
> Victor Dodon.

D28679: [KPropertiesDialog] Disable changing dir icon on samba shares

[Ahmad Samir]

ahmadsamir planned changes to this revision.

REPOSITORY
  R241 KIO

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

To: ahmadsamir, #frameworks, dfaure, sitter
Cc: broulik, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D28679: [KPropertiesDialog] Disable changing dir icon on samba shares

[Harald Sitter]

sitter added a comment.


  Hm. So, this is a bit complicated I've noticed.
  
  Let's consider the following cases:
  
  - `desktop:` is not a local file but can set and read dir icons without 
penalty
  - `camera:` is not a local file AND `Class=:local` BUT (I think?) cannot set 
dir icons as it is `writing=false` (in practice the slave seems to list dirs 
with `rwx` though, so I'm not sure, may be a slave bug)
  - `smb:` is not a local file AND not local class nor do we want to have icons 
for it
  
  With that in mind I believe Kai's approach would be more correct here as 
otherwise the desktop: case doesn't work. Perhaps it'd make also sense to check 
`!KProtocolInfo::supportsWriting`. The way I see it if a slave implementation 
doesn't support writing... then it doesn't support writing. The directory being 
writable or not wouldn't change the lack of write support in the slave.

REPOSITORY
  R241 KIO

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

To: ahmadsamir, #frameworks, dfaure, sitter
Cc: broulik, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D28679: [KPropertiesDialog] Disable changing dir icon on samba shares

[Ahmad Samir]

ahmadsamir updated this revision to Diff 79643.
ahmadsamir added a comment.


  --verbatim

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28679?vs=79640&id=79643

BRANCH
  l-kprop (branched from master)

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

AFFECTED FILES
  src/widgets/kpropertiesdialog.cpp

To: ahmadsamir, #frameworks, dfaure, sitter
Cc: broulik, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D28679: [KPropertiesDialog] Disable changing dir icon on samba shares

[Ahmad Samir]

ahmadsamir added a comment.


  I thought about that, but since I am not experienced with network mounts (I 
rarely use them), I didn't expand the test
  
  In D28679#644274 , @sitter wrote:
  
  > Looks reasonable. Though... Shouldn't that rather be any protocol that 
isn't `file`? Or at least all that are remote? (assuming we have a way of 
telling which slaves are remote)
  >
  > If I open sftp it also shows no dir icons yet lets me set one.
  
  
  I thought about that, but since I am not experienced with network mounts (I 
rarely use them), I didn't venture that way.
  
  About checking, KFileItem has an isLocalFile() method.

REPOSITORY
  R241 KIO

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

To: ahmadsamir, #frameworks, dfaure, sitter
Cc: broulik, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D28679: [KPropertiesDialog] Disable changing dir icon on samba shares

[Kai Uwe Broulik]

broulik added a comment.


  > assuming we have a way of telling which slaves are remote
  
  `KProtocolInfo::protocolClass(url.scheme())` not being `":local"`

REPOSITORY
  R241 KIO

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

To: ahmadsamir, #frameworks, dfaure, sitter
Cc: broulik, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D28679: [KPropertiesDialog] Disable changing dir icon on samba shares

[Harald Sitter]

sitter added a comment.


  Looks reasonable. Though... Shouldn't that rather be any protocol that isn't 
`file`? Or at least all that are remote? (assuming we have a way of telling 
which slaves are remote)
  
  If I open sftp it also shows no dir icons yet lets me set one.

REPOSITORY
  R241 KIO

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

To: ahmadsamir, #frameworks, dfaure, sitter
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

KDE CI: Frameworks » kirigami » kf5-qt5 SUSEQt5.12 - Build # 414 - Fixed!

[CI System]

BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks/job/kirigami/job/kf5-qt5%20SUSEQt5.12/414/
 Project:
kf5-qt5 SUSEQt5.12
 Date of build:
Wed, 08 Apr 2020 13:41:13 +
 Build duration:
2 min 16 sec and counting
   BUILD ARTIFACTS
  abi-compatibility-results.yamlacc/KF5Kirigami2-5.69.0.xmlcompat_reports/KF5Kirigami2_compat_report.htmllogs/KF5Kirigami2/5.69.0/log.txt
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot.home.jenkins.workspace.Frameworks.kirigami.kf5-qt5_SUSEQt512 Failed: 0 test(s), Passed: 5 test(s), Skipped: 0 test(s), Total: 5 test(s)
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report60%
(3/5)41%
(13/32)41%
(13/32)41%
(1397/3381)27%
(609/2275)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(1/1)100%
(1/1)100%
(1/1)100%
(0/0)examples.applicationitemapp0%
(0/1)0%
(0/1)0%
(0/8)100%
(0/0)src53%
(9/17)53%
(9/17)38%
(925/2413)25%
(390/1560)src.libkirigami50%
(3/6)50%
(3/6)73%
(471/647)39%
(219/567)src.scenegraph0%
(0/7)0%
(0/7)0%
(0/312)0%
(0/148)

D28679: [KPropertiesDialog] Disable changing dir icon on samba shares

[Ahmad Samir]

ahmadsamir added a comment.


  I don't have access to a samba share so I tested with an sftp:// one, after 
modifying the diff locally.

REPOSITORY
  R241 KIO

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

To: ahmadsamir, #frameworks, dfaure, sitter
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D28679: [KPropertiesDialog] Disable changing dir icon on samba shares

[Ahmad Samir]

ahmadsamir created this revision.
ahmadsamir added reviewers: Frameworks, dfaure, sitter.
Herald added a project: Frameworks.
ahmadsamir requested review of this revision.

REVISION SUMMARY
  Changing the icon of a dir on a samba share doesn't seem to have an
  effect, this is intentional as reading .directory would impact network
  resources negatively, see the bug report for more details.
  
  BUG: 205954

TEST PLAN
  make && ctest

REPOSITORY
  R241 KIO

BRANCH
  l-kprop (branched from master)

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

AFFECTED FILES
  src/widgets/kpropertiesdialog.cpp

To: ahmadsamir, #frameworks, dfaure, sitter
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D28336: Drop klauncher usage from KCrash

[David Edmundson]

davidedmundson updated this revision to Diff 79634.
davidedmundson added a comment.


  and merge methods

REPOSITORY
  R285 KCrash

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28336?vs=78636&id=79634

BRANCH
  master

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

AFFECTED FILES
  src/kcrash.cpp
  src/kcrash.h

To: davidedmundson, sitter
Cc: sitter, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D28336: Drop klauncher usage from KCrash

[David Edmundson]

This revision was automatically updated to reflect the committed changes.
Closed by commit R285:d8e41e64b7d0: Drop klauncher usage from KCrash (authored 
by davidedmundson).

REPOSITORY
  R285 KCrash

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28336?vs=79634&id=79635

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

AFFECTED FILES
  src/kcrash.cpp
  src/kcrash.h

To: davidedmundson, sitter
Cc: sitter, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D28336: Drop klauncher usage from KCrash

[Harald Sitter]

sitter accepted this revision.
sitter added inline comments.
This revision is now accepted and ready to land.

INLINE COMMENTS

> kcrash.cpp:627
>  
>  void KCrash::startProcess(int argc, const char *argv[], bool waitAndExit)
>  {

This seems to serve no purpose anymore. startProcessInternal could be merged 
into this so there's only one startProcess function left.

REPOSITORY
  R285 KCrash

BRANCH
  master

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

To: davidedmundson, sitter
Cc: sitter, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D28669: make CopyJob non-recursive

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> meven wrote in copyjob.cpp:814
> about @anthonyfieroni comment
> Add `&& !isKilled()` with a code path to handle it properly.

It'll not help, message queue will be blocked and you kill the from other 
thread, which is not what we want.

REPOSITORY
  R241 KIO

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

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

D28675: [KMimeTypeChooser] Add the ability to filter the treeview with a QSFPM

[Ahmad Samir]

ahmadsamir created this revision.
ahmadsamir added reviewers: Frameworks, cfeck, dfaure.
Herald added a project: Frameworks.
ahmadsamir requested review of this revision.

REVISION SUMMARY
  - Add each mimetype comment as a tooltip instead of an optional column; 
adjust the docs accordingly
  - Add an updated screenshot with current KDE Breeze style
  - Add a sizeHint() override, which uses QFontMetric::averageCharWidth() to 
get a "sensibly"-sized dialog
  
  BUG: 245637
  FIXED-IN: 5.70

TEST PLAN
  The kmimetypechoosertest app still works

REPOSITORY
  R236 KWidgetsAddons

BRANCH
  l-kmimechooser (branched from master)

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

AFFECTED FILES
  docs/pics/kmimetypechooserdialog.png
  src/kmimetypechooser.cpp
  src/kmimetypechooser.h
  tests/kmimetypechoosertest.cpp

To: ahmadsamir, #frameworks, cfeck, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D28673: [PackageUrlInterceptor] Make QRegularExpression static

[Pino Toscano]

pino added a comment.


  Does it really need to be a regular expression, though? A simple string 
search & replace should do the same thing.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: broulik, #plasma
Cc: pino, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D28673: [PackageUrlInterceptor] Make QRegularExpression static

[Kai Uwe Broulik]

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

REVISION SUMMARY
  Avoids recompiling it every time.

TEST PLAN
  Called 27 times on plasmashell startup for me, saves 90% of time in here

REPOSITORY
  R242 Plasma Framework (Library)

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

AFFECTED FILES
  src/plasmaquick/packageurlinterceptor.cpp

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

D28669: make CopyJob non-recursive

[Méven Car]

meven requested changes to this revision.
meven added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> copyjob.cpp:814
>  Q_Q(CopyJob);
> -if (m_currentStatSrc != m_srcList.constEnd()) {
> +while (m_currentStatSrc != m_srcList.constEnd()) {
>  m_currentSrcURL = (*m_currentStatSrc);

about @anthonyfieroni comment
Add `&& !isKilled()` with a code path to handle it properly.

> copyjob.cpp:892
>  if (that) {
>  statNextSrc();// we could use a loop instead of a 
> recursive call :)
>  }

I guess we can update this comment

REPOSITORY
  R241 KIO

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

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