D15718: Do not index the path if the path has no execute permissions.

[James Smith]

smithjd abandoned this revision.

REPOSITORY
  R293 Baloo

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

To: smithjd, ngraham, #baloo, ltoscano
Cc: ltoscano, marten, bruns, ngraham, kde-frameworks-devel, #baloo, 
ashaposhnikov, michaelh, astippich, spoorun, abrahams

D15718: Do not index the path if the path has no execute permissions.

[James Smith]

smithjd added a comment.


  xattrs are no good for vaults because the xattr is ignored for the 
mountpoint, only restored when the volume is unmounted.

REPOSITORY
  R293 Baloo

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

To: smithjd, ngraham, #baloo, ltoscano
Cc: ltoscano, marten, bruns, ngraham, kde-frameworks-devel, #baloo, 
ashaposhnikov, michaelh, astippich, spoorun, abrahams

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

[Albert Astals Cid]

aacid added a comment.


  In D14631#331827 , @emateli wrote:
  
  > @aacid Would be great if you'd point me towards making them installable. 
Only the dialog itself should be exported.
  
  
  install(FILES
  
${KIOWidgets_HEADERS}
${CMAKE_CURRENT_BINARY_DIR}/kiowidgets_export.h
DESTINATION ${KDE_INSTALL_INCLUDEDIR_KF5}/KIOWidgets COMPONENT Devel)
  
  in src/widgets/CMakeLists.txt

REPOSITORY
  R241 KIO

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

To: emateli, #frameworks, dfaure
Cc: asensi, rkflx, dfaure, aacid, ngraham, kde-frameworks-devel, michaelh, bruns

D15714: add a string suffix to test data and use for unicode testing of taglibwriter

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> taglibwritertest.cpp:60
>  
> -QCOMPARE(extractedTitle, QStringLiteral("Title1"));
> -QCOMPARE(extractedArtist, QStringLiteral("Artist1"));
> -QCOMPARE(extractedAlbum, QStringLiteral("Album1"));
> +QCOMPARE(extractedTitle, QString(QStringLiteral("Title1") + 
> stringSuffix.toUtf8()));
> +QCOMPARE(extractedArtist, QString(QStringLiteral("Artist1") + 
> stringSuffix.toUtf8()));

I don't think it is a good idea to do a `stringSuffix.toUtf8().fromUtf8()` 
round trip here for each test case and for each tag.

> taglibwritertest.cpp:73
>  {
> +QString unicodeTestStringSuffix = QStringLiteral("€");
>  QTest::addColumn("fileType");

If safeguarding against bad editors is really necessary, better do it here.
Alternatively, you can use C++11 unicode string literals, e.g.
`QString(u"€")`
If you save that one as e.g. latin1 and try to compile it, gcc reports:

  error: converting to execution character set: Invalid or incomplete multibyte 
or wide character
   QString a{u"�"};

REPOSITORY
  R286 KFileMetaData

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

To: astippich, mgallien, bruns
Cc: smithjd, svuorela, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, bruns, abrahams

D15739: [Places panel] Don't show Root by default

[Nathaniel Graham]

ngraham added a comment.


  Thanks @bruns!  So it looks like the Windows code can go after all. 
@davidedmundson?

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg, davidedmundson
Cc: bruns, davidedmundson, abetts, svenmauch, broulik, acrouthamel, 
kde-frameworks-devel, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Stefan Brüns]

bruns added a comment.


  In D15739#331727 , @ngraham wrote:
  
  > @bruns, do you know if Solid (and by extension, the Places panel) shows 
Windows disks when Dolphin is run on Windows?
  >
  > Side note: does anyone run Dolphin on Windows? Do we distribute it at all?
  
  
  As @davidedmundson said/hinted, Solid uses 
https://docs.microsoft.com/en-us/windows/desktop/api/fileapi/nf-fileapi-getlogicaldrives
 to show the drives in the Devices section (same as done using e.g. UDisks2 on 
Linux), but this is independent of listing *pathes* (Root, Home) in the Places 
section.

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg, davidedmundson
Cc: bruns, davidedmundson, abetts, svenmauch, broulik, acrouthamel, 
kde-frameworks-devel, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Stefan Brüns]

bruns added a comment.


  In D15739#331700 , @davidedmundson 
wrote:
  
  > They don't.
  >
  > Why are you changing the if (windows) code path above the elif?
  
  
  Note this is `_WIN32_WCE`, i.e. dead code. WCE is no more supported by any 
half way recent Qt version.

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg, davidedmundson
Cc: bruns, davidedmundson, abetts, svenmauch, broulik, acrouthamel, 
kde-frameworks-devel, michaelh, ngraham

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

[Emirald Mateli]

emateli added a comment.


  In D14631#303886 , @aacid wrote:
  
  > As far as i can see none of these headers get installed so should they all 
be renamed to _p.h ?
  >
  > Also if the headers don't get installed how do you use the new classes?
  
  
  @aacid Would be great if you'd point me towards making them installable. Only 
the dialog itself should be exported.
  @dfaure Do functions for getting file suffix and name already exist in KIO or 
other KDE libraries? The whole filenameutils feels like duplicate work.
  
  I want to give KDevelop a spin for this, how do I import a new formatter so 
it can format my code according to the KDE guidelines.

INLINE COMMENTS

> dfaure wrote in batchrenametypes.h:32
> This looks more like a namespace than an actual class, given that everything 
> is static.
> 
> Alternatively (and even better), make capturedGroups and the two associated 
> methods non-static, meaning that one has to instanciate the class in order to 
> use it. This is only used in the dialog, right? So there's no need for this 
> "global" variable, it can just be a member of BatchRenameTypes which can be a 
> member of the dialog, AFAICS.

I tried the instance route but then I wouldn't be able to pass the method as a 
callback. Current way is not ideal either. Will figure something out.

REPOSITORY
  R241 KIO

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

To: emateli, #frameworks, dfaure
Cc: asensi, rkflx, dfaure, aacid, ngraham, kde-frameworks-devel, michaelh, bruns

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

[Emirald Mateli]

emateli updated this revision to Diff 42324.
emateli marked 15 inline comments as done.
emateli added a comment.


  - Code style fixes
  - use .insert instead of []
  - Indent Q_OBJECT
  - use isEmpty instead of len==0
  - use aggregate initialization
  - protected -> private
  - added parent check
  - use functions instead of QString fields
  - delete itemData
  - code format
  - FileNameUtils converted to namespace
  - remove unnecessary using

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14631?vs=39137=42324

BRANCH
  batchrename2

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

AFFECTED FILES
  autotests/CMakeLists.txt
  autotests/batchrenametypestest.cpp
  autotests/batchrenametypestest.h
  src/widgets/CMakeLists.txt
  src/widgets/rename/batchrenamedialog.cpp
  src/widgets/rename/batchrenamedialog.h
  src/widgets/rename/batchrenamedialogmodel.cpp
  src/widgets/rename/batchrenamedialogmodel.h
  src/widgets/rename/batchrenametypes.cpp
  src/widgets/rename/batchrenametypes.h
  src/widgets/rename/batchrenamevar.cpp
  src/widgets/rename/batchrenamevar.h
  src/widgets/rename/filenameutils.cpp
  src/widgets/rename/filenameutils.h
  tests/CMakeLists.txt
  tests/batchrenamedialogtest_gui.cpp

To: emateli, #frameworks, dfaure
Cc: asensi, rkflx, dfaure, aacid, ngraham, kde-frameworks-devel, michaelh, bruns

D15721: Make lock on plasmavault icon visible with breeze-dark

[Nathaniel Graham]

ngraham added a comment.


  In D15721#331785 , @ndavis wrote:
  
  > Thanks, I really like that series. It gives a real feeling of forward 
momentum for KDE.
  
  
  Yep, that's one of the big reasons why I do it. :)

REPOSITORY
  R266 Breeze Icons

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

To: ndavis, #vdg, ngraham
Cc: ngraham, abetts, kde-frameworks-devel, michaelh, bruns

D15721: Make lock on plasmavault icon visible with breeze-dark

[Noah Davis]

ndavis added a comment.


  In D15721#331723 , @ngraham wrote:
  
  > FYI, this earned a place in next week's Usability & Productivity report 
. :)
  
  
  Thanks, I really like that series. It gives a real feeling of forward 
momentum for KDE.

REPOSITORY
  R266 Breeze Icons

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

To: ndavis, #vdg, ngraham
Cc: ngraham, abetts, kde-frameworks-devel, michaelh, bruns

D15740: Use non deprecated fastInsert in baloo

[Jaime Torres Amate]

This revision was automatically updated to reflect the committed changes.
Closed by commit R293:a931be1d7e3a: Use non deprecated fastInsert in baloo 
(authored by jtamate).

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15740?vs=42283=42315

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

AFFECTED FILES
  src/kioslaves/search/kio_search.cpp

To: jtamate, dfaure, #frameworks, broulik, #baloo, ngraham
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

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

[Jaime Torres Amate]

This revision was automatically updated to reflect the committed changes.
Closed by commit R293:95af521127c1: Use String to store UDS_USER and UDS_GROUP 
of String type. (authored by jtamate).

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15635?vs=42044=42314

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

AFFECTED FILES
  src/kioslaves/search/kio_search.cpp
  src/kioslaves/search/kio_search.h

To: jtamate, dfaure, #baloo, #frameworks, ngraham
Cc: ngraham, broulik, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, 
spoorun, bruns, abrahams

D15745: Implement Media and MediaEndpoint API

[Manuel Weichselbaumer]

mweichselbaumer updated this revision to Diff 42313.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15745?vs=42300=42313

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

AFFECTED FILES
  src/CMakeLists.txt
  src/a2dp-codecs.h
  src/interfaces/org.bluez.Media1.xml
  src/interfaces/org.bluez.MediaEndpoint1.xml
  src/media.cpp
  src/media.h
  src/media_p.cpp
  src/media_p.h
  src/mediaendpoint.cpp
  src/mediaendpoint.h
  src/mediaendpoint_p.cpp
  src/mediaendpoint_p.h
  src/mediaendpointadaptor.cpp
  src/mediaendpointadaptor.h
  src/pendingcall.h

To: mweichselbaumer, drosca
Cc: broulik, kde-frameworks-devel, michaelh, ngraham, bruns

D15739: [Places panel] Don't show Root by default

[Nathaniel Graham]

ngraham added a subscriber: bruns.
ngraham added a comment.


  @bruns, do you know if Solid (and by extension, the Places panel) shows 
Windows disks when Dolphin is run on Windows?
  
  Side note: does anyone run Dolphin on Windows? Do we distribute it at all?

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg, davidedmundson
Cc: bruns, davidedmundson, abetts, svenmauch, broulik, acrouthamel, 
kde-frameworks-devel, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[David Edmundson]

davidedmundson added a comment.


  No idea.
  
  It isn't the same as adding root as it adds every drive quite deliberately.
  
  There is a solid windows device back end, but it needs asking someone.

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg, davidedmundson
Cc: davidedmundson, abetts, svenmauch, broulik, acrouthamel, 
kde-frameworks-devel, michaelh, ngraham, bruns

D11880: Add firewall-config and firewall-applet icons

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.


  Never mind, it's a Cuttlefish issue. Everything looks good to me.
  
  Do we have final sign-off by other #VDG 
 folks?

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

D15721: Make lock on plasmavault icon visible with breeze-dark

[Nathaniel Graham]

ngraham added a comment.


  FYI, this earned a place in next week's Usability & Productivity report 
. :)

REPOSITORY
  R266 Breeze Icons

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

To: ndavis, #vdg, ngraham
Cc: ngraham, abetts, kde-frameworks-devel, michaelh, bruns

KDE CI: Frameworks » breeze-icons » kf5-qt5 SUSEQt5.9 - Build # 53 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/breeze-icons/job/kf5-qt5%20SUSEQt5.9/53/
 Project:
kf5-qt5 SUSEQt5.9
 Date of build:
Tue, 25 Sep 2018 16:51:05 +
 Build duration:
4 min 17 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 4 test(s), Skipped: 0 test(s), Total: 5 test(s)Failed: TestSuite.scalable
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report100%
(2/2)83%
(5/6)83%
(5/6)34%
(102/299)20%
(36/182)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsdefault100%
(1/1)100%
(1/1)74%
(42/57)56%
(10/18)autotests80%
(4/5)80%
(4/5)25%
(60/242)16%
(26/164)

KDE CI: Frameworks » breeze-icons » kf5-qt5 SUSEQt5.10 - Build # 109 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/breeze-icons/job/kf5-qt5%20SUSEQt5.10/109/
 Project:
kf5-qt5 SUSEQt5.10
 Date of build:
Tue, 25 Sep 2018 16:51:05 +
 Build duration:
1 min 18 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 4 test(s), Skipped: 0 test(s), Total: 5 test(s)Failed: TestSuite.scalable
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report100%
(2/2)83%
(5/6)83%
(5/6)34%
(102/299)20%
(36/182)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsdefault100%
(1/1)100%
(1/1)74%
(42/57)56%
(10/18)autotests80%
(4/5)80%
(4/5)25%
(60/242)16%
(26/164)

D15721: Make lock on plasmavault icon visible with breeze-dark

[Nathaniel Graham]

This revision was automatically updated to reflect the committed changes.
Closed by commit R266:f729894087c6: Make lock on plasmavault icon visible with 
breeze-dark (authored by ndavis, committed by ngraham).

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15721?vs=42213=42312

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

AFFECTED FILES
  icons-dark/apps/16/plasmavault.svg
  icons-dark/apps/22/plasmavault.svg
  icons/apps/16/plasmavault.svg
  icons/apps/22/plasmavault.svg

To: ndavis, #vdg, ngraham
Cc: ngraham, abetts, kde-frameworks-devel, michaelh, bruns

D15721: Make lock on plasmavault icon visible with breeze-dark

[Nathaniel Graham]

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


  Ah thanks. For some reason Cuttlefish was not displaying the icon properly. :/
  
  Can confirm the problem and that this fixes it! Will land the patch shortly.

REPOSITORY
  R266 Breeze Icons

BRANCH
  plasmavault-icon-fix (branched from master)

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

To: ndavis, #vdg, ngraham
Cc: ngraham, abetts, kde-frameworks-devel, michaelh, bruns

D15745: Implement Media and MediaEndpoint API

[Manuel Weichselbaumer]

mweichselbaumer marked 5 inline comments as done.
mweichselbaumer added inline comments.

INLINE COMMENTS

> broulik wrote in media_p.h:37
> Any particular reason this class must inherit `QObject`, you don't seem to be 
> using `signal` or `slot` in it

MediaPrivate will later act as parent for child objects (inheriting QObject).

REPOSITORY
  R269 BluezQt

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

To: mweichselbaumer, drosca
Cc: broulik, kde-frameworks-devel, michaelh, ngraham, bruns

D15718: Do not index the path if the path has no execute permissions.

[Nathaniel Graham]

ngraham added a comment.


  So the goal is to be able to turn off indexing for certain files?
  
  Baloo already has a global toggle to stop indexing anything, and you can also 
add folders to the exclude lists.
  
  But stepping back even farther: //why// do we want to turn off indexing for 
certain files? Is it because they might be dangerous? If that's the case, then 
we should work on sandboxing the file extractor, not break the functionality. 
We never intentionally break our software because it might be dangerous; we fix 
the danger, or offer a warning. Our users are adults and prefer to be treated 
as such.
  
  Perhaps we should get some more eyes on D8532: [WIP] Restrict file extractor 
with Seccomp .

REPOSITORY
  R293 Baloo

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

To: smithjd, ngraham, #baloo, ltoscano
Cc: ltoscano, marten, bruns, ngraham, kde-frameworks-devel, #baloo, 
ashaposhnikov, michaelh, astippich, spoorun, abrahams

D15644: Provide option to hide menu bar for Ksysguard

[Nathaniel Graham]

ngraham added a comment.


  Actually, reading over this again, is it really necessary to add a 
`showMessage` parameter to `toggleShowMenuBar`? In general bool-only arguments 
are frowned upon because they're not very readable; enums are preferred in 
their place. But do we even need that parameter in the first place? I don't see 
that it's ever even set to false anywhere.

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

To: lsartorelli, ngraham, #plasma, #frameworks
Cc: acrouthamel, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D15402: [Thumbnails] Paint larger "one thumbnail" tile only when needed

[Nathaniel Graham]

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


  Seems fine to me. Code looks sane and behavior appears to be unchanged. Maybe 
faster, but that might just be confirmation bias talking. :)

REPOSITORY
  R320 KIO Extras

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

To: broulik, #dolphin, ngraham
Cc: anthonyfieroni, kde-frameworks-devel, kfm-devel, feverfew, michaelh, 
spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp

D15739: [Places panel] Don't show Root by default

[Nathaniel Graham]

ngraham added a comment.


  In D15739#331700 , @davidedmundson 
wrote:
  
  > They don't.
  >
  > Why are you changing the if (windows) code path above the elif?
  
  
  Because that's the windows equivalent of adding Root. I reasoned that if we 
don't want to show a Places panel entry for Root on Linux, then for similar 
reasons we wouldn't want to create Places panel entries for Windows disks when 
on Windows. Don't disks show up in the Devices section on Windows like they do 
on Linux? Can't test, sorry.

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg, davidedmundson
Cc: davidedmundson, abetts, svenmauch, broulik, acrouthamel, 
kde-frameworks-devel, michaelh, ngraham, bruns

D15739: [Places panel] Don't show Root by default

[David Edmundson]

davidedmundson requested changes to this revision.
davidedmundson added a comment.
This revision now requires changes to proceed.


  They don't.
  
  Why are you changing the if (windows) code path above the elif?

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg, davidedmundson
Cc: davidedmundson, abetts, svenmauch, broulik, acrouthamel, 
kde-frameworks-devel, michaelh, ngraham, bruns

D15739: [Places panel] Don't show Root by default

[Nathaniel Graham]

ngraham added a comment.


  Do four +1s amount to any formal Accept statuses? ;-)

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg
Cc: abetts, svenmauch, broulik, acrouthamel, kde-frameworks-devel, michaelh, 
ngraham, bruns

D11880: Add firewall-config and firewall-applet icons

[Nathaniel Graham]

ngraham added a comment.


  Hmm, I still get the issue in Cuttlefish after clearing the cache.
  
  Is anyone else able to reproduce this issue with viewing the icons in 
Cuttlefish and turning on "Inverted"?

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

D15644: Provide option to hide menu bar for Ksysguard

[Luca Sartorelli]

lsartorelli added a comment.


  Thank you very much,
  
  I am so happy and proud for my first patch.
  
  Here my details:
  Name: Luca 
  Surname: Sartorelli 
  Mail: dj3...@gmail.com
  
  And here is the patch for gwenview: https://phabricator.kde.org/D15747
  
  Just added you as reviewer 2 secs ago

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

To: lsartorelli, ngraham, #plasma, #frameworks
Cc: acrouthamel, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D15741: Use correct MaximumSize

[Nathaniel Graham]

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

REPOSITORY
  R320 KIO Extras

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

To: broulik, #frameworks, ngraham
Cc: kde-frameworks-devel, kfm-devel, feverfew, michaelh, spoorun, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp

D15745: Implement Media and MediaEndpoint API

[David Rosca]

drosca added inline comments.

INLINE COMMENTS

> broulik wrote in media_p.h:35
> I think this needs `Q_DECL_HIDDEN`

Why? This class is not exported by default, afaik it is only needed if 
MediaPrivate was declared inside Media class (eg. Media::MediaPrivate), which 
it is not.

REPOSITORY
  R269 BluezQt

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

To: mweichselbaumer, drosca
Cc: broulik, kde-frameworks-devel, michaelh, ngraham, bruns

D15644: Provide option to hide menu bar for Ksysguard

[Nathaniel Graham]

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


  Wonderful. I think the warning is good enough for now until we can come up 
with a better way of handling this. Code looks good! Can you provide your full 
name and email address so I can land this patch with proper authorship 
information?
  
  Now that you're an expert on creating warning messages when the menu bar is 
hidden, would you like to try your hand at doing the same thing for Gwenview? 
https://bugs.kde.org/show_bug.cgi?id=210620 ;-)
  
  Any concerns from #plasma  or 
#frameworks  folks before I land 
this?

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

To: lsartorelli, ngraham, #plasma, #frameworks
Cc: acrouthamel, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D15740: Use non deprecated fastInsert in baloo

[Nathaniel Graham]

ngraham accepted this revision.

REPOSITORY
  R293 Baloo

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

To: jtamate, dfaure, #frameworks, broulik, #baloo, ngraham
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D15740: Use non deprecated fastInsert in baloo

[Nathaniel Graham]

ngraham added a reviewer: Baloo.

REPOSITORY
  R293 Baloo

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

To: jtamate, dfaure, #frameworks, broulik, #baloo
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D15745: Implement Media and MediaEndpoint API

[Kai Uwe Broulik]

broulik added inline comments.

INLINE COMMENTS

> media_p.h:35
> +
> +class MediaPrivate : public QObject
> +{

I think this needs `Q_DECL_HIDDEN`

> media_p.h:37
> +{
> +Q_OBJECT
> +

Any particular reason this class must inherit `QObject`, you don't seem to be 
using `signal` or `slot` in it

REPOSITORY
  R269 BluezQt

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

To: mweichselbaumer, drosca
Cc: broulik, kde-frameworks-devel, michaelh, ngraham, bruns

D15739: [Places panel] Don't show Root by default

[Andres Betts]

abetts added a comment.


  +1

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg
Cc: abetts, svenmauch, broulik, acrouthamel, kde-frameworks-devel, michaelh, 
ngraham, bruns

D15745: Implement Media and MediaEndpoint API

[David Rosca]

drosca requested changes to this revision.
drosca added a comment.
This revision now requires changes to proceed.


  Looks good apart from the coding style.
  Also it would be great to have at least basic autotest.

INLINE COMMENTS

> media.h:95
> +
> +friend class MediaPrivate;
> +};

Not needed

> media_p.h:42
> +
> +Media *q;
> +BluezMedia *m_bluezMedia;

`= nullptr`

> media_p.h:43
> +Media *q;
> +BluezMedia *m_bluezMedia;
> +};

Same as above

> mediaendpoint.h:83
> + */
> +virtual void setConfiguration(const QString& transportObjectPath, const 
> QVariantMap& properties);
> +

Coding style: `const QString `
Please change everywhere

> mediaendpoint.h:114
> +private:
> +friend class Media;
> +};

Not needed

> mediaendpoint_p.h:37
> +
> +} // namepsace BluezQt
> +

typo "namepsace" -> "namespace"

REPOSITORY
  R269 BluezQt

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

To: mweichselbaumer, drosca
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15745: Implement Media and MediaEndpoint API

[Kai Uwe Broulik]

broulik added a reviewer: drosca.

REPOSITORY
  R269 BluezQt

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

To: mweichselbaumer, drosca
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15745: Implement Media and MediaEndpoint API

[Manuel Weichselbaumer]

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

REPOSITORY
  R269 BluezQt

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

AFFECTED FILES
  src/CMakeLists.txt
  src/interfaces/org.bluez.Media1.xml
  src/interfaces/org.bluez.MediaEndpoint1.xml
  src/media.cpp
  src/media.h
  src/media_p.cpp
  src/media_p.h
  src/mediaendpoint.cpp
  src/mediaendpoint.h
  src/mediaendpoint_p.h
  src/mediaendpointadaptor.cpp
  src/mediaendpointadaptor.h
  src/pendingcall.h

To: mweichselbaumer
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15740: Use non deprecated fastInsert in baloo

[Kai Uwe Broulik]

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

REPOSITORY
  R293 Baloo

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

To: jtamate, dfaure, #frameworks, broulik
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D15644: Provide option to hide menu bar for Ksysguard

[Luca Sartorelli]

lsartorelli updated this revision to Diff 42298.
lsartorelli added a comment.


  Added remainder message box with keyboard shortcut, to have back the menu bar.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15644?vs=42101=42298

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

AFFECTED FILES
  gui/ksysguard.cpp
  gui/ksysguard.h

To: lsartorelli, ngraham, #plasma, #frameworks
Cc: acrouthamel, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D15741: Use correct MaximumSize

[Kai Uwe Broulik]

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

REVISION SUMMARY
  The maximum size for local thumbnails was raised form 5 MB to unlimited 
elsewhere.
  
  BUG: 238690

TEST PLAN
  I get more thumbnails for folders now, especially videos

REPOSITORY
  R320 KIO Extras

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

AFFECTED FILES
  thumbnail/thumbnail.cpp

To: broulik, #frameworks, ngraham
Cc: kde-frameworks-devel, kfm-devel, feverfew, michaelh, spoorun, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp

D15721: Make lock on plasmavault icon visible with breeze-dark

[Noah Davis]

ndavis added a comment.


  In D15721#331388 , @ngraham wrote:
  
  > Can you provide steps to reproduce the problem so I can test it? In my 
naive testing, the `plasmavault` icon looks like a lock and shows up fine on 
both Breeze Light and Breeze Dark.
  
  
  Here's one way to reproduce the bug that this change fixes:
  
  1. Switch to the Breeze Dark icon theme
  2. Use the `plasmavault` icon for a favorite place in Dolphin. In this case, 
it's the `~/Vaults` folder.
  
  Here's a close-up of the problem.F6284027: Screenshot_20180925_045058.png 

  
  > I'm also confused by the images you posted (which should be in the Test 
Plan{ section, BTW that depict different icons. Can you help a brotha out?
  
  The pictures are just showing how the icon looks after the change. I suppose 
I should have shown the old version for comparison. I don't actually know what 
the test plan section is for and I don't know where I would find that 
information.

REPOSITORY
  R266 Breeze Icons

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

To: ndavis, #vdg
Cc: ngraham, abetts, kde-frameworks-devel, michaelh, bruns

D15402: [Thumbnails] Paint larger "one thumbnail" tile only when needed

[Anthony Fieroni]

anthonyfieroni added a comment.


  Looks good +1

REPOSITORY
  R320 KIO Extras

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

To: broulik, #dolphin, ngraham
Cc: anthonyfieroni, kde-frameworks-devel, kfm-devel, feverfew, michaelh, 
spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp

D7446: [Places panel] Add a Recently Used item by default

[Sven Mauch]

svenmauch added a comment.


  While I wouldn't use it (yet?) I think it's a great addition and would 
benefit a lot of people. I give it a +1, especially if D15739 
 gets approved aswell. ;)

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, broulik, elvisangelaccio, markg, #vdg, #frameworks
Cc: svenmauch, kde-frameworks-devel, spoorun, anthonyfieroni, andreaska, 
gregormi, markg, alexeymin, broulik, elvisangelaccio, dfaure, davidedmundson, 
ltoscano, #konqueror, feverfew, michaelh, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp

D15402: [Thumbnails] Paint larger "one thumbnail" tile only when needed

[Kai Uwe Broulik]

broulik added a comment.
Herald added projects: Dolphin, Frameworks.
Herald added subscribers: kfm-devel, kde-frameworks-devel.


  Ping

REPOSITORY
  R320 KIO Extras

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

To: broulik, #dolphin, ngraham
Cc: kde-frameworks-devel, kfm-devel, feverfew, michaelh, spoorun, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp

D15739: [Places panel] Don't show Root by default

[Sven Mauch]

svenmauch added a comment.


  All arguments make sense. It even looks a lot better without the red folder 
icon. +1

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg
Cc: svenmauch, broulik, acrouthamel, kde-frameworks-devel, michaelh, ngraham, 
bruns

D15740: Use non deprecated fastInsert in baloo

[Jaime Torres Amate]

jtamate created this revision.
jtamate added reviewers: dfaure, Frameworks.
Herald added projects: Frameworks, Baloo.
Herald added subscribers: Baloo, kde-frameworks-devel.
jtamate requested review of this revision.

REVISION SUMMARY
  Replace the deprecated uds insert method by fastInsert.

REPOSITORY
  R293 Baloo

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

AFFECTED FILES
  src/kioslaves/search/kio_search.cpp

To: jtamate, dfaure, #frameworks
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D15739: [Places panel] Don't show Root by default

[Kai Uwe Broulik]

broulik added a comment.


  +1 it's an entry I always hide when I setup someone's computer

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg
Cc: broulik, acrouthamel, kde-frameworks-devel, michaelh, ngraham, bruns

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

[David Faure]

dfaure accepted this revision.

REPOSITORY
  R293 Baloo

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

To: jtamate, dfaure, #baloo, #frameworks, ngraham
Cc: ngraham, broulik, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, 
spoorun, bruns, abrahams