T11542: Remove KHTML

[Méven Car]

meven closed subtask T11543: Port Konqueror away from KHTML as 
"Resolved".

TASK DETAIL
  https://phabricator.kde.org/T11542

To: meven
Cc: cordlandwehr, ngraham, #konqueror, #plasma, #okular, #kde_applications, 
#frameworks, knauss, hannahk, davidre, GB_2, ahmadsamir, kpiwowarski, usta, 
asturmlechner, jucato, cfeck, cgiboudeaux, cullmann, vkrause

T11542: Remove KHTML

[Méven Car]

meven closed subtask T11540: Port kio-extras/man away from KHTML as 
"Resolved".

TASK DETAIL
  https://phabricator.kde.org/T11542

To: meven
Cc: cordlandwehr, ngraham, #konqueror, #plasma, #okular, #kde_applications, 
#frameworks, knauss, hannahk, davidre, GB_2, ahmadsamir, kpiwowarski, usta, 
asturmlechner, jucato, cfeck, cgiboudeaux, cullmann, vkrause

T12641: Refactor KFileProtocol::copy

[Méven Car]

meven added a comment.


  Found some hints how to help the kernel manage its buffers and page cache 
efficiently when writing
  http://lkml.iu.edu/hypermail/linux/kernel/1005.2/01845.html
  http://lkml.iu.edu/hypermail/linux/kernel/1005.2/01953.html
  
  Also additional fadvise might be worse testing.
  
(void)posix_fadvise(fd, 0, 0, POSIX_FADV_WILLNEED);
(void)posix_fadvise(fd, 0, 0, POSIX_FADV_SEQUENTIAL);
(void)posix_fadvise(fd, 0, 0, POSIX_FADV_NOREUSE);
  
  In any case, I desperately need a benchmark tool as kioclient can't really 
play that role.
  Something that would link directly with file IOSLAVE.

TASK DETAIL
  https://phabricator.kde.org/T12641

To: meven
Cc: apol, dfaure, #frameworks, #dolphin, ognarb, broulik, meven, sdorishlab, 
badbunny, waitquietly, azyx, nikolaik, pberestov, iasensio, aprcela, fprice, 
LeGast00n, koehn, cblack, fbampaloukas, alexde, Codezela, feverfew, michaelh, 
ahmadsamir, spoorun, navarromorales, firef, ngraham, andrebarros, bruns, 
emmanuelp, rdieter, mikesomov, vkrause

T12641: Refactor KFileProtocol::copy

[Méven Car]

meven added a comment.


  I have a first step : split to two loops use copy_file_range
  
  https://invent.kde.org/frameworks/kio/-/merge_requests/602

TASK DETAIL
  https://phabricator.kde.org/T12641

To: meven
Cc: apol, dfaure, #frameworks, #dolphin, ognarb, broulik, meven, sdorishlab, 
badbunny, waitquietly, azyx, nikolaik, pberestov, iasensio, aprcela, fprice, 
LeGast00n, koehn, cblack, fbampaloukas, alexde, Codezela, feverfew, michaelh, 
ahmadsamir, spoorun, navarromorales, firef, ngraham, andrebarros, bruns, 
emmanuelp, rdieter, mikesomov, vkrause

D26600: Refactor fstab handling

[Méven Car]

meven added a comment.


  Would be great to continue this.

REPOSITORY
  R245 Solid

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

To: hallas, #frameworks, bruns, meven
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ahmadsamir, ngraham, 
bruns, vkrause

T12641: Refactor KFileProtocol::copy

[Méven Car]

meven added a comment.


  The io_uring cp example is a good reference to make use of the API.
  https://github.com/axboe/liburing/blob/master/examples/io_uring-cp.c

TASK DETAIL
  https://phabricator.kde.org/T12641

To: meven
Cc: apol, dfaure, #frameworks, #dolphin, ognarb, broulik, meven, sdorishlab, 
badbunny, waitquietly, azyx, nikolaik, pberestov, iasensio, aprcela, fprice, 
LeGast00n, koehn, cblack, fbampaloukas, alexde, Codezela, feverfew, michaelh, 
ahmadsamir, spoorun, navarromorales, firef, ngraham, andrebarros, bruns, 
emmanuelp, rdieter, mikesomov, vkrause

T12641: Refactor KFileProtocol::copy

[Méven Car]

meven added a comment.


  copy https://github.com/coreutils/coreutils/blob/master/src/copy.c#L301 use 
https://man7.org/linux/man-pages/man2/copy_file_range.2.html or 
https://github.com/coreutils/gnulib/blob/master/lib/full-write.h
  Did not know about copy_file_range, it was introduced in kernel 4.5 and 
improved/reworked in 5.3 apparently.
  It is simply newer than sendfile (but older than io_uring), it seems to be 
just almost a drop-in replacement for sendfile with reflink aka COW support for 
supporting fs built-in.
  That should be the next area for investigation, it is way simpler than our 
current reflink support, but it is Linux only.

TASK DETAIL
  https://phabricator.kde.org/T12641

To: meven
Cc: apol, dfaure, #frameworks, #dolphin, ognarb, broulik, meven, sdorishlab, 
badbunny, waitquietly, azyx, nikolaik, pberestov, iasensio, aprcela, fprice, 
LeGast00n, koehn, cblack, fbampaloukas, alexde, Codezela, feverfew, michaelh, 
ahmadsamir, spoorun, navarromorales, firef, ngraham, andrebarros, bruns, 
emmanuelp, rdieter, mikesomov, vkrause

D29525: Make Previews devicePixelRatio aware

[Méven Car]

meven abandoned this revision.
meven added a comment.


  In D29525#677057 , 
@elvisangelaccio wrote:
  
  > Superseded by https://invent.kde.org/system/dolphin/-/merge_requests/147
  >
  > @meven Can you abandon this one?
  
  
  Sure, god forgotten.

REPOSITORY
  R318 Dolphin

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

To: meven, #dolphin, #frameworks, dfaure, ngraham, elvisangelaccio
Cc: kfm-devel, badbunny, waitquietly, azyx, nikolaik, pberestov, iasensio, 
aprcela, fprice, fbampaloukas, alexde, Codezela, feverfew, meven, spoorun, 
navarromorales, firef, ngraham, andrebarros, emmanuelp, rdieter, mikesomov

D29397: KPreviewJob : Support for DeviceRatioPixel

[Méven Car]

meven abandoned this revision.
meven added a comment.


  With the xdg spec now covering this use case 
https://gitlab.freedesktop.org/xdg/xdg-specs/-/merge_requests/40
  I have reworked this into 
https://invent.kde.org/frameworks/kio/-/merge_requests/266

REPOSITORY
  R241 KIO

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

To: meven, dfaure, broulik, #frameworks, ngraham
Cc: bruns, elvisangelaccio, kossebau, davidedmundson, kde-frameworks-devel, 
LeGast00n, cblack, michaelh, ngraham

T8349: Improve Places panel usability and presentation

[Méven Car]

meven updated the task description.

TASK DETAIL
  https://phabricator.kde.org/T8349

To: ngraham, meven
Cc: ahmadsamir, sitter, #frameworks, tomsk, bruns, michaelh, acrouthamel, 
sharvey, mmustac, jtamate, rkflx, #dolphin, ngraham, badbunny, fabiogomes, 
waitquietly, azyx, dmenig, nikolaik, pberestov, manueljlin, iasensio, Orage, 
aprcela, fprice, cblack, konkinartem, ian, jguidon, Ghost6, jraleigh, 
fbampaloukas, squeakypancakes, alexde, IohannesPetros, GB_2, Codezela, 
feverfew, trickyricky26, meven, crozbo, spoorun, ndavis, navarromorales, firef, 
andrebarros, skadinna, emmanuelp, rdieter, mikesomov, aaronhoneycutt, mbohlender

D29397: KPreviewJob : Support for DeviceRatioPixel

[Méven Car]

meven added a comment.


  Specification work is so slow I am hesitant to bypass it for now and work on 
code, even if it means resync with spec later.
  https://gitlab.freedesktop.org/xdg/xdg-specs/-/merge_requests/35

REPOSITORY
  R241 KIO

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

To: meven, dfaure, broulik, #frameworks, ngraham
Cc: bruns, elvisangelaccio, kossebau, davidedmundson, kde-frameworks-devel, 
LeGast00n, cblack, michaelh, ngraham

D22764: Stabilize test KFileWidgetTest::testDropFile

[Méven Car]

meven added inline comments.

INLINE COMMENTS

> dfaure wrote in kfilewidgettest.cpp:481
> For the record, this is broken, it should have been `completed(QUrl)`. 
> Specifying argument names is incorrect in SIGNAL() and SLOT() macros.
> 
> It makes the wait() below fail every time, but after 5s, so the 100ms became 
> 5s :-)
> 
> I'm working on fixing this.

Let me know if I should relay you.

REPOSITORY
  R241 KIO

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

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

D26424: [kdiroperator] Add method for accessing actions without KActionCollection

[Méven Car]

meven added a comment.


  ping @nicolasfella

INLINE COMMENTS

> kdiroperator.h:425
>   */
>  KActionCollection *actionCollection() const;
>  

Mark it deprecated / add a // KF6 TODO remove ?

REPOSITORY
  R241 KIO

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

To: nicolasfella, #frameworks, dfaure
Cc: meven, dhaumann, aacid, kde-frameworks-devel, LeGast00n, cblack, michaelh, 
ngraham, bruns

D26407: KFileItem: Improve isSlow to not block when a network mount is unresponsive, make SkipMimeTypeFromContent skip only on slow fs

[Méven Car]

meven abandoned this revision.
meven marked 5 inline comments as done.
meven added a comment.


  Moved to https://invent.kde.org/frameworks/kio/-/merge_requests/174

REPOSITORY
  R241 KIO

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

To: meven, #frameworks, ngraham, broulik, dfaure
Cc: cfeck, anthonyfieroni, kde-frameworks-devel, LeGast00n, cblack, michaelh, 
ngraham, bruns

D29526: Thumbnails: make thumbnail generation dpr-aware

[Méven Car]

meven added a comment.


  In D29526#676386 , @bruns wrote:
  
  > For all but text the DPR is completely irrelevant, large@1 is identical to 
normal@2.
  
  
  Yes and that's up to thumbnail creators to decide. To take advantage of this, 
we would need to introduce some ThumbnailCreator type that would say whether or 
not generated thumbnail might be influenced by DPR (i.e) text. That would 
necessitate change the ThumbnailCreator API.
  
  But the implementation will stay a lot simpler if we don't this level of 
complexity and adding will have a limited interest. Storing twice large@1 would 
happen only when a user would change DPR, thumbnails cache size limit will stay 
enforced.

REPOSITORY
  R320 KIO Extras

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

To: meven, #frameworks, dfaure, broulik, sitter, ngraham, bruns
Cc: bruns, kde-frameworks-devel, kfm-devel, waitquietly, azyx, nikolaik, 
pberestov, iasensio, aprcela, fprice, LeGast00n, cblack, fbampaloukas, alexde, 
Codezela, feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, emmanuelp, rdieter, mikesomov

D29397: KPreviewJob : Support for DeviceRatioPixel

[Méven Car]

meven added a comment.


  FYI on this patch serie, I am currently focusing on the specification work : 
https://gitlab.freedesktop.org/xdg/xdg-specs/-/merge_requests/35
  Then I am thinking about moving my patch serie to gitlab reflecting on the 
last feedback of course.
  Those patchs are getting old and are going to need some degree of rebasing.

INLINE COMMENTS

> meven wrote in previewjob.cpp:754
> Maybe add it optionally, when different from 1

Not needed.

> bruns wrote in previewjob.cpp:699
> mixing floating point and integers for a memory-allocation like function is a 
> real bad idea.

I am thinking about making devicePixelRatio an int.

> previewjob.h:196
> + *
> + * @since 5.71
> + */

update

> dfaure wrote in thumbcreator.h:191
> 5.70 is tagged already

update

REPOSITORY
  R241 KIO

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

To: meven, dfaure, broulik, #frameworks, ngraham
Cc: bruns, elvisangelaccio, kossebau, davidedmundson, kde-frameworks-devel, 
LeGast00n, cblack, michaelh, ngraham

D29526: Thumbnails: make thumbnail generation dpr-aware

[Méven Car]

meven added a comment.


  I just sent a specification evolution to cover this use case : 
https://gitlab.freedesktop.org/xdg/xdg-specs/-/merge_requests/35

REPOSITORY
  R320 KIO Extras

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

To: meven, #frameworks, dfaure, broulik, sitter, ngraham
Cc: kde-frameworks-devel, kfm-devel, waitquietly, azyx, nikolaik, pberestov, 
iasensio, aprcela, fprice, LeGast00n, cblack, fbampaloukas, alexde, Codezela, 
feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, rdieter, mikesomov

D28856: Save disabling of desktop file components in kglobalshortcutsrc

[Méven Car]

meven added a comment.


  ping
  
  Would be important to land with 
https://invent.kde.org/frameworks/kglobalaccel/-/merge_requests/2

REPOSITORY
  R268 KGlobalAccel

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

To: davidre, davidedmundson, fvogt, meven
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D28745: Skip caching thumbnails on encrypted filesystems

[Méven Car]

meven added a comment.


  In D28745#675033 , @marcingu wrote:
  
  > Ok, so, what I want to do now is to create static method `findByPath` which 
is going to return Solid::StorageVolume instance (is there a case in which we 
can expect something different than StorageVolume?).
  
  
  I don't think so, Storage Volume is any block device.
  This function may fail if the input path is not valid for instance.
  
  > Should it be `StorageVolume Device::findByPath(QString)` or rather 
`StorageVolume StorageVolume::findByPath(QString)`?
  
  My preference is `StorageVolume Device::findByPath(QString)`, to keep all 
entry points to Solid in Device and somewhat similar to the `Device::listFrom*` 
functions.
  
  > For implementation itself I want to create structure with mountpoints and 
StorageVolumes which will be updated if new Device is added/removed and we 
learn this via Solid notifications.
  
  Yeah that's what I imagined as well, but this is already taken care of by the 
base DeviceManager.
  
  > I am thinking it should either be part of `DeviceManagerStorage` or 
separate class similar to DeviceManagerStorage. Not sure which.
  
  Either way you can simply wrap the base DeviceManager.
  Your method should only be a utility function not adding new Structures, 
everything is in Solid already.
  
  > I don't know how to get a mountpoint for StorageVolume.
  
  You make me realize you should filter StorageVolume (any block device) that 
are also StorageAccess (anything that can be mounted) which has the mountpoints 
in `filePath()`.
  
  https://api.kde.org/frameworks/solid/html/classSolid_1_1StorageVolume.html 
  https://api.kde.org/frameworks/solid/html/classSolid_1_1StorageAccess.html
  
  > What do you think about it?
  
  I like it

REPOSITORY
  R320 KIO Extras

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

To: marcingu, ivan, broulik, #dolphin, ngraham, meven, bruns
Cc: thiago, bruns, meven, ngraham, kde-frameworks-devel, kfm-devel, 
waitquietly, azyx, nikolaik, pberestov, iasensio, aprcela, fprice, LeGast00n, 
cblack, fbampaloukas, alexde, Codezela, feverfew, michaelh, spoorun, 
navarromorales, firef, andrebarros, emmanuelp, rdieter, mikesomov

D28745: Skip caching thumbnails on encrypted filesystems

[Méven Car]

meven added a comment.


  In D28745#674827 , @bruns wrote:
  
  > In D28745#674711 , @meven wrote:
  >
  > >
  >
  >
  >
  >
  > > Solid does not provide straight `folder => StorageVolume` yet, but I 
think Solid could have such a utility feature added.
  > >  Something like `Solid::Device::findByPath()`, it would need to 
canonically and recursively resolves the path parent to pay attention to 
symlinks.
  > >  This would also help D26407 
  >
  > No recursion needed, `stat` provides the device.
  
  
  Only when the file is not a symlink, if so we need to check the symlink 
target recursively, that's what I meant.
  
  > And can you please use arc to upload the patch - it is nearly impossible to 
do a review with the missing context
  
  Or pushing it to https://community.kde.org/Infrastructure/GitLab

REPOSITORY
  R320 KIO Extras

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

To: marcingu, ivan, broulik, #dolphin, ngraham, meven, bruns
Cc: thiago, bruns, meven, ngraham, kde-frameworks-devel, kfm-devel, 
waitquietly, azyx, nikolaik, pberestov, iasensio, aprcela, fprice, LeGast00n, 
cblack, fbampaloukas, alexde, Codezela, feverfew, michaelh, spoorun, 
navarromorales, firef, andrebarros, emmanuelp, rdieter, mikesomov

D28745: Skip caching thumbnails on encrypted filesystems

[Méven Car]

meven added a comment.


  In D28745#674294 , @marcingu wrote:
  
  > I tried to research Solid using api.kde.org 
(https://api.kde.org/frameworks/solid/html/classSolid_1_1Device.html, 
https://api.kde.org/frameworks/solid/html/classSolid_1_1StorageVolume.html) and 
looking for usages of both Solid::Device and Solid::StorageVolume in code but 
I'm not able to get StorageVolume instance for given file/directory.
  
  
  The good starting point would be the main Solid Tutorial : 
  https://techbase.kde.org/Development/Tutorials/Solid/Device_Discovery
  
  Solid does not provide straight `folder => StorageVolume` yet, but I think 
Solid could have such a utility feature added.
  Something like `Solid::Device::findByPath()`, it would need to canonically 
and recursively resolves the path parent to pay attention to symlinks.
  This would also help D26407 
  
  KMountPoint::findByPath has it although it is not perfect. Compared to solid 
it does not refresh automatically and can't send signals and does not know 
about encryption.
  
  > Could someone help me with that?
  
  Sure. I have been dreaming about such feature for a while.

REPOSITORY
  R320 KIO Extras

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

To: marcingu, ivan, broulik, #dolphin, ngraham, meven, bruns
Cc: thiago, bruns, meven, ngraham, kde-frameworks-devel, kfm-devel, 
waitquietly, azyx, nikolaik, pberestov, iasensio, aprcela, fprice, LeGast00n, 
cblack, fbampaloukas, alexde, Codezela, feverfew, michaelh, spoorun, 
navarromorales, firef, andrebarros, emmanuelp, rdieter, mikesomov

D29774: Add X-KDE-DBUS-Restricted-Interfaces to Application desktop entry fields

[Méven Car]

meven added a comment.


  In D29774#674668 , @zzag wrote:
  
  > > This added field to Application desktop entries allows to declare an 
access request to a DBUS interface.
  >
  > Just to be sure. Does X-KDE-DBUS-Restricted-Interfaces indicate a D-Bus 
interface or a D-Bus service?
  
  
  A string that designates a DBus interface name such as what is done in KWin 
side D29407 .
  
  So for spectacle it will be :
  
  `X-KDE-DBUS-Restricted-Interfaces=org_kde_kwin_effect-screenshot`

REPOSITORY
  R309 KService

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

To: meven, davidedmundson, zzag, #frameworks, #kwin, ngraham
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29774: Add X-KDE-DBUS-Restricted-Interfaces to Application desktop entry fields

[Méven Car]

This revision was automatically updated to reflect the committed changes.
Closed by commit R309:cdad4f73ccb7: Add X-KDE-DBUS-Restricted-Interfaces to 
Application desktop entry fields (authored by meven).

REPOSITORY
  R309 KService

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29774?vs=82926&id=83222

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

AFFECTED FILES
  src/services/application.desktop

To: meven, davidedmundson, zzag, #frameworks, #kwin, ngraham
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29397: KPreviewJob : Support for DeviceRatioPixel

[Méven Car]

meven added a comment.


  In D29397#674365 , @kossebau wrote:
  
  > To not have that block this improvement, you could in parallel for now use 
a "kde" namespaced directories, say "*@kde2x/", where we/you could just use the 
for-now-KF-only theme extension. And once there is an agreed specification 
extension, the code could be switched to just any non-namespace shared hidpi 
folder matching whatever the specification ended up to be.
  
  
  Well I don't think it is needed, since those @2x are not used by anyone as 
far as I can tell.
  I'd rather move on here without the spec finalized and then update the 
implementation once it is finalized.
  I don't expect it to be much different from the current implementation, and I 
hope this implementation serve the specification process since we will have an 
implementation to debate on.
  And this will save two commits that serves no purpose IMO, we are not Web 
Browsers after all, we are not breaking anyones business.
  
  Alternatively I'd rather wait for the spec to be finalized.
  Just to avoid the friction.

REPOSITORY
  R241 KIO

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

To: meven, dfaure, broulik, #frameworks, ngraham
Cc: elvisangelaccio, kossebau, davidedmundson, kde-frameworks-devel, LeGast00n, 
cblack, michaelh, ngraham, bruns

D29525: Make Previews devicePixelRatio aware

[Méven Car]

meven edited the summary of this revision.

REPOSITORY
  R318 Dolphin

BRANCH
  arcpatch-D29525

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

To: meven, #dolphin, #frameworks, dfaure, ngraham, elvisangelaccio
Cc: kfm-devel, waitquietly, azyx, nikolaik, pberestov, iasensio, aprcela, 
fprice, fbampaloukas, alexde, Codezela, feverfew, meven, spoorun, 
navarromorales, firef, ngraham, andrebarros, emmanuelp, rdieter, mikesomov

D29805: Thumbnail djvu: Avoid a crash when djvu is not installed

[Méven Car]

meven edited the summary of this revision.

REPOSITORY
  R320 KIO Extras

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

To: meven, #frameworks, broulik, ngraham, pino
Cc: pino, kde-frameworks-devel, kfm-devel, waitquietly, azyx, nikolaik, 
pberestov, iasensio, aprcela, fprice, LeGast00n, cblack, fbampaloukas, alexde, 
Codezela, feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, rdieter, mikesomov

D29805: Thumbnail djvu: Avoid a crash when djvu is not installed

[Méven Car]

meven added a comment.


  In D29805#674225 , @pino wrote:
  
  > In D29805#674218 , @meven wrote:
  >
  > > In D29805#674206 , @pino wrote:
  > >
  > > > In D29805#674205 , @meven 
wrote:
  > > >
  > > > > In D29805#674204 , @pino 
wrote:
  > > > >
  > > > > > > FIXED-IN: 20.08
  > > > > >
  > > > > > still for 20.08...
  > > > >
  > > > >
  > > > > Yes kio-extra is released with KDE  Applications
  > > >
  > > >
  > > > Yes, I know. Asked to land this fix instead to `release/20.04`, and 
thus change the commit message accordingly.
  > >
  > >
  > > Please be explicit when you comment, no one could deduce what you meant.
  >
  >
  > I wrote it **two** times to land this on the stable branch: the first time 
when I explained why the initial patch was not correct and what the problem 
actually was (with hints on short term and long term fixes), and earlier today 
when I wrote:
  >
  > In D29805#674185 , @pino wrote:
  >
  > > please target `release/20.04` for this fix, thanks
  >
  >
  > There is no need to "deduce" anything, just read what I wrote, thanks.
  >
  > > If this is ready approve and add a comment "land to 20.04".
  >
  > The commit message still says "20.08", so not yet.
  
  
  That's a detail once it is accepted it is trivial to edit...
  Usually this is not required.

REPOSITORY
  R320 KIO Extras

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

To: meven, #frameworks, broulik, ngraham, pino
Cc: pino, kde-frameworks-devel, kfm-devel, waitquietly, azyx, nikolaik, 
pberestov, iasensio, aprcela, fprice, LeGast00n, cblack, fbampaloukas, alexde, 
Codezela, feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, rdieter, mikesomov

D29805: Thumbnail djvu: Avoid a crash when djvu is not installed

[Méven Car]

meven added a comment.


  In D29805#674206 , @pino wrote:
  
  > In D29805#674205 , @meven wrote:
  >
  > > In D29805#674204 , @pino wrote:
  > >
  > > > > FIXED-IN: 20.08
  > > >
  > > > still for 20.08...
  > >
  > >
  > > Yes kio-extra is released with KDE  Applications
  >
  >
  > Yes, I know. Asked to land this fix instead to `release/20.04`, and thus 
change the commit message accordingly.
  
  
  Please be explicit when you comment, no one could deduce what you meant.
  
  If this is ready approve and add a comment "land to 20.04".

REPOSITORY
  R320 KIO Extras

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

To: meven, #frameworks, broulik, ngraham, pino
Cc: pino, kde-frameworks-devel, kfm-devel, waitquietly, azyx, nikolaik, 
pberestov, iasensio, aprcela, fprice, LeGast00n, cblack, fbampaloukas, alexde, 
Codezela, feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, rdieter, mikesomov

D29805: Thumbnail djvu: Avoid a crash when djvu is not installed

[Méven Car]

meven added a comment.


  In D29805#674204 , @pino wrote:
  
  > > FIXED-IN: 20.08
  >
  > still for 20.08...
  
  
  Yes kio-extra is released with KDE  Applications

REPOSITORY
  R320 KIO Extras

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

To: meven, #frameworks, broulik, ngraham, pino
Cc: pino, kde-frameworks-devel, kfm-devel, waitquietly, azyx, nikolaik, 
pberestov, iasensio, aprcela, fprice, LeGast00n, cblack, fbampaloukas, alexde, 
Codezela, feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, rdieter, mikesomov

D29805: Thumbnail djvu: Avoid a crash when djvu is not installed

[Méven Car]

meven marked 2 inline comments as done.

REPOSITORY
  R320 KIO Extras

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

To: meven, #frameworks, broulik, ngraham, pino
Cc: pino, kde-frameworks-devel, kfm-devel, waitquietly, azyx, nikolaik, 
pberestov, iasensio, aprcela, fprice, LeGast00n, cblack, fbampaloukas, alexde, 
Codezela, feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, rdieter, mikesomov

D29805: Thumbnail djvu: Avoid a crash when djvu is not installed

[Méven Car]

meven updated this revision to Diff 83177.
meven marked an inline comment as done.
meven added a comment.


  Remove brackets

REPOSITORY
  R320 KIO Extras

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29805?vs=83018&id=83177

BRANCH
  arcpatch-D29805

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

AFFECTED FILES
  thumbnail/djvucreator.cpp

To: meven, #frameworks, broulik, ngraham, pino
Cc: pino, kde-frameworks-devel, kfm-devel, waitquietly, azyx, nikolaik, 
pberestov, iasensio, aprcela, fprice, LeGast00n, cblack, fbampaloukas, alexde, 
Codezela, feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, rdieter, mikesomov

D29805: Thumbnail djvu: Avoid a crash when djvu is not installed

[Méven Car]

meven marked an inline comment as done.
meven added inline comments.

INLINE COMMENTS

> pino wrote in djvucreator.cpp:52-54
> extra brackets

I'd like to use instead the Framework coding style to improve homogenizing 
coding styles.
https://community.kde.org/Policies/Frameworks_Coding_Style#Braces

REPOSITORY
  R320 KIO Extras

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

To: meven, #frameworks, broulik, ngraham, pino
Cc: pino, kde-frameworks-devel, kfm-devel, waitquietly, azyx, nikolaik, 
pberestov, iasensio, aprcela, fprice, LeGast00n, cblack, fbampaloukas, alexde, 
Codezela, feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, rdieter, mikesomov

D26113: Places: Use Solid::Device::DisplayName for DisplayRole

[Méven Car]

meven closed this revision.

REPOSITORY
  R241 KIO

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

To: meven, #frameworks, ngraham, dfaure
Cc: dfaure, feverfew, bruns, broulik, kde-frameworks-devel, LeGast00n, cblack, 
michaelh, ngraham

D3835: [Calendar] Populate overview models on-demand

[Méven Car]

meven accepted this revision as: meven.
meven added a comment.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.


  @broulik is it still on the table ?

REPOSITORY
  R242 Plasma Framework (Library)

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

To: broulik, #plasma, mart, meven
Cc: kde-frameworks-devel, meven, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, michaelh, 
ZrenBot, ngraham, bruns, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, ahiemstra, mart

D26113: Places: Use Solid::Device::DisplayName for DisplayRole

[Méven Car]

meven added a comment.


  In D26113#674129 , @dfaure wrote:
  
  > Please check that kfileplacesmodeltest and kfileplacesviewtest still pass.
  
  
  Just did, nothing changed, it is expected : only fstab declared filesystem or 
manually mounted fs display text is changing and this is not tested here.
  
  > (they fail here with baloosearch: stuff for some reason, I didn't 
investigate; but it passes on CI)
  
  I am fixing this in https://invent.kde.org/frameworks/kio/-/merge_requests/25

REPOSITORY
  R241 KIO

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

To: meven, #frameworks, ngraham
Cc: dfaure, feverfew, bruns, broulik, kde-frameworks-devel, LeGast00n, cblack, 
michaelh, ngraham

D29805: Thumbnail djvu: Avoid a crash when djvu is not installed

[Méven Car]

meven added a comment.


  ping @pino

REPOSITORY
  R320 KIO Extras

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

To: meven, #frameworks, broulik, ngraham, pino
Cc: pino, kde-frameworks-devel, kfm-devel, waitquietly, azyx, nikolaik, 
pberestov, iasensio, aprcela, fprice, LeGast00n, cblack, fbampaloukas, alexde, 
Codezela, feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, rdieter, mikesomov

D29806: Thumbnail folders: scale down sub image when their height is too big

[Méven Car]

meven abandoned this revision.
meven added a comment.


  Moved to https://invent.kde.org/network/kio-extras/-/merge_requests/2

REPOSITORY
  R320 KIO Extras

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

To: meven, #frameworks, ngraham
Cc: kde-frameworks-devel, kfm-devel, waitquietly, azyx, nikolaik, pberestov, 
iasensio, aprcela, fprice, LeGast00n, cblack, fbampaloukas, alexde, Codezela, 
feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, rdieter, mikesomov

D26113: Places: Use Solid::Device::DisplayName for DisplayRole

[Méven Car]

meven edited the summary of this revision.

REPOSITORY
  R241 KIO

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

To: meven, #frameworks, ngraham
Cc: dfaure, feverfew, bruns, broulik, kde-frameworks-devel, LeGast00n, cblack, 
michaelh, ngraham

D26113: Places: Use Solid::Device::DisplayName for DisplayRole

[Méven Car]

meven added a subscriber: dfaure.
meven added a comment.


  This should be ready to land @dfaure @ngraham
  
  (D26114  will land after KF 5.71 release)

REPOSITORY
  R241 KIO

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

To: meven, #frameworks, ngraham
Cc: dfaure, feverfew, bruns, broulik, kde-frameworks-devel, LeGast00n, cblack, 
michaelh, ngraham

D26113: Places: Use Solid::Device::DisplayName for DisplayRole

[Méven Car]

meven updated this revision to Diff 83162.
meven added a comment.


  Update to new Solid::Device::DisplayName

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26113?vs=79277&id=83162

BRANCH
  arcpatch-D26113

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

AFFECTED FILES
  src/filewidgets/kfileplacesitem.cpp

To: meven, #frameworks, ngraham
Cc: feverfew, bruns, broulik, kde-frameworks-devel, LeGast00n, cblack, 
michaelh, ngraham

D29461: Fix kio-extras build on Windows

[Méven Car]

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

REPOSITORY
  R320 KIO Extras

BRANCH
  arcpatch-D29461

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

To: brute4s99, vonreth, meven
Cc: sitter, meven, kde-frameworks-devel, kfm-devel, waitquietly, azyx, 
nikolaik, pberestov, iasensio, aprcela, fprice, LeGast00n, cblack, 
fbampaloukas, alexde, Codezela, feverfew, michaelh, spoorun, navarromorales, 
firef, ngraham, andrebarros, bruns, emmanuelp, rdieter, mikesomov

D29397: KPreviewJob : Support for DeviceRatioPixel

[Méven Car]

meven added inline comments.

INLINE COMMENTS

> ngraham wrote in previewjob.cpp:401
> What about if I'm using a 250% scale factor? Maybe there should be an `@3x` 
> folder too.

or just trunate devicePixelRatio to int

REPOSITORY
  R241 KIO

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

To: meven, dfaure, broulik, #frameworks, ngraham
Cc: elvisangelaccio, kossebau, davidedmundson, kde-frameworks-devel, LeGast00n, 
cblack, michaelh, ngraham, bruns

D28590: Add a QString Solid::Device::displayName, used in Fstab Device for network mounts

[Méven Car]

meven added a comment.


  The patch currently does not work.

INLINE COMMENTS

> ervin wrote in device.h:99
> Why not have a default implementation which returns descriptions()? This 
> would make for a less intrusive patch (I think it's in part what @bruns 
> suggested earlier).

Well I have to make this virtual it seems so this call is dynamically 
dispatched by `return_SOLID_CALL(Ifaces::Device *, d->backendObject(), 
QString(), displayName());`
I assumed this would work based on my review comments rather than on tests :/

REPOSITORY
  R245 Solid

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

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

D26113: Places: Use Solid::Device::DisplayName for DisplayRole

[Méven Car]

meven retitled this revision from "Places: For mounted volume display mount 
points instead of description" to "Places: Use Solid::Device::DisplayName for 
DisplayRole".

REPOSITORY
  R241 KIO

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

To: meven, #frameworks, ngraham
Cc: feverfew, bruns, broulik, kde-frameworks-devel, LeGast00n, cblack, 
michaelh, ngraham

D26113: Places: For mounted volume display mount points instead of description

[Méven Car]

meven edited the summary of this revision.

REPOSITORY
  R241 KIO

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

To: meven, #frameworks, ngraham
Cc: feverfew, bruns, broulik, kde-frameworks-devel, LeGast00n, cblack, 
michaelh, ngraham

D28590: Add a QString Solid::Device::displayName, used in Fstab Device for network mounts

[Méven Car]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit R245:cdbfb3e799c7: Add a QString Solid::Device::displayName, 
used in Fstab Device for network… (authored by meven).

REPOSITORY
  R245 Solid

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28590?vs=83128&id=83146

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

AFFECTED FILES
  src/solid/devices/backends/fstab/fstabdevice.cpp
  src/solid/devices/backends/fstab/fstabdevice.h
  src/solid/devices/frontend/device.cpp
  src/solid/devices/frontend/device.h
  src/solid/devices/ifaces/device.cpp
  src/solid/devices/ifaces/device.h

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

D29397: KPreviewJob : Support for DeviceRatioPixel

[Méven Car]

meven marked an inline comment as done.
meven added a comment.


  In D29397#673758 , @ngraham wrote:
  
  > Overall seems sane. Two questions though:
  
  
  
  
  > Is this @2x suffix standardized?
  
  No but it is already being used for icon caching.
  
  > What happens if I'm using 125% scaling, generate some previews, and then 
switch back to 100% (no scaling?)
  
  When your screen is scaled 125%, the application gets a dpr of 2.
  When you go back to 100%. the thumbnails in `~/.cache/thumbnails/large` and 
`~/.cache/thumbnails/normal` will be used again.
  Meaning you will have two sets of previews.
  
  > The images using a pixel ratio are stored in cache in 
~/.cache/thumbnails/normal@2x for 256*256 (i.e 128*128@2x) and 
~/.cache/thumbnails/large@2x for 512*512 (i.e 256*256@2x)

REPOSITORY
  R241 KIO

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

To: meven, dfaure, broulik, #frameworks, ngraham
Cc: elvisangelaccio, kossebau, davidedmundson, kde-frameworks-devel, LeGast00n, 
cblack, michaelh, ngraham, bruns

D29397: KPreviewJob : Support for DeviceRatioPixel

[Méven Car]

meven updated this revision to Diff 83140.
meven added a comment.


  Rebasing

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29397?vs=83139&id=83140

BRANCH
  arcpatch-D29397

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

AFFECTED FILES
  src/widgets/previewjob.cpp
  src/widgets/previewjob.h
  src/widgets/thumbcreator.cpp
  src/widgets/thumbcreator.h

To: meven, dfaure, broulik, #frameworks, ngraham
Cc: elvisangelaccio, kossebau, davidedmundson, kde-frameworks-devel, LeGast00n, 
cblack, michaelh, ngraham, bruns

D29397: KPreviewJob : Support for DeviceRatioPixel

[Méven Car]

meven updated this revision to Diff 83139.
meven marked 3 inline comments as done.
meven added a comment.


  Typo, const, doxygen fix

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29397?vs=82709&id=83139

BRANCH
  arcpatch-D29397

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

AFFECTED FILES
  src/widgets/previewjob.cpp
  src/widgets/previewjob.h
  src/widgets/thumbcreator.cpp
  src/widgets/thumbcreator.h

To: meven, dfaure, broulik, #frameworks, ngraham
Cc: elvisangelaccio, kossebau, davidedmundson, kde-frameworks-devel, LeGast00n, 
cblack, michaelh, ngraham, bruns

D27731: Improve KDirModel to avoid showing '+' if there are no subdirs

[Méven Car]

meven added a comment.


  I believe this causes https://bugs.kde.org/show_bug.cgi?id=419434
  It causes similar issue with dolphin and KFileWidget
  
  This is not a correct behavior.
  It prevents file treeview to show files within folders with no subfolders.
  The bug has a proposed patch :
  https://bugsfiles.kde.org/attachment.cgi?id=128272
  
  I think this needs to be reverted or at least reworked.

REPOSITORY
  R241 KIO

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

To: dfaure, apol, ahmadsamir, meven, rrosch
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D28590: Add a QString Solid::Device::displayName, used in Fstab Device for network mounts

[Méven Car]

meven added a comment.


  In D28590#673609 , @dfaure wrote:
  
  > @meven you're confusing me with my clone @ervin.
  
  
  lol
  I thought this was still you when I read the comment.
  Thanks @ervin

REPOSITORY
  R245 Solid

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

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

D28590: Add a QString Solid::Device::displayName, used in Fstab Device for network mounts

[Méven Car]

meven added a comment.


  This makes the patch way less intrusive in the process, thanks @dfaure

REPOSITORY
  R245 Solid

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

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

D28590: Add a QString Solid::Device::displayName, used in Fstab Device for network mounts

[Méven Car]

meven updated this revision to Diff 83128.
meven added a comment.


  Remove unneeded change

REPOSITORY
  R245 Solid

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28590?vs=83127&id=83128

BRANCH
  arcpatch-D28590_1

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

AFFECTED FILES
  src/solid/devices/backends/fstab/fstabdevice.cpp
  src/solid/devices/backends/fstab/fstabdevice.h
  src/solid/devices/frontend/device.cpp
  src/solid/devices/frontend/device.h
  src/solid/devices/ifaces/device.cpp
  src/solid/devices/ifaces/device.h

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

D28590: Add a QString Solid::Device::displayName, used in Fstab Device for network mounts

[Méven Car]

meven updated this revision to Diff 83127.
meven marked 5 inline comments as done.
meven added a comment.


  Use a default implementation for Device::displayName()

REPOSITORY
  R245 Solid

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28590?vs=83112&id=83127

BRANCH
  arcpatch-D28590_1

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

AFFECTED FILES
  src/solid/devices/backends/fstab/fstabdevice.cpp
  src/solid/devices/backends/fstab/fstabdevice.h
  src/solid/devices/backends/udisks2/udisksdevicebackend.cpp
  src/solid/devices/frontend/device.cpp
  src/solid/devices/frontend/device.h
  src/solid/devices/ifaces/device.cpp
  src/solid/devices/ifaces/device.h

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

D28590: Add a QString Solid::Device::displayName, used in Fstab Device for network mounts

[Méven Car]

meven added subscribers: broulik, dfaure.
meven added a comment.


  Maybe @dfaure or @broulik would be nice to help the review effort.

REPOSITORY
  R245 Solid

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

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

D29281: Deprecate defunct functions

[Méven Car]

meven accepted this revision as: meven.
meven added a comment.
This revision is now accepted and ready to land.


  Nice

REPOSITORY
  R308 KRunner

BRANCH
  deprecations (branched from master)

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

To: alex, #plasma, broulik, davidedmundson, vkrause, meven
Cc: kossebau, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29281: Deprecate defunct functions

[Méven Car]

meven added inline comments.

INLINE COMMENTS

> querymatch.cpp:338
>  {
> -return d->runner && d->runner.data()->hasRunOptions();
>  }

Did you really need to remove this code ?

> querymatch.cpp:343
>  
> +#if KRUNNER_BUILD_DEPRECATED_SINCE(5, 1)
>  void QueryMatch::createConfigurationInterface(QWidget *parent)

#if KRUNNER_BUILD_DEPRECATED_SINCE(5, 71)

REPOSITORY
  R308 KRunner

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

To: alex, #plasma, broulik, davidedmundson, vkrause, meven
Cc: kossebau, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29805: Thumbnail djvu: Avoid a crash when djvu is not installed

[Méven Car]

meven added a comment.


  @pino ping

REPOSITORY
  R320 KIO Extras

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

To: meven, #frameworks, broulik, ngraham, pino
Cc: pino, kde-frameworks-devel, kfm-devel, waitquietly, azyx, nikolaik, 
pberestov, iasensio, aprcela, fprice, LeGast00n, cblack, fbampaloukas, alexde, 
Codezela, feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, rdieter, mikesomov

D28590: Add a QString Solid::Device::displayName, used in Fstab Device for network mounts

[Méven Car]

meven updated this revision to Diff 83112.
meven added a comment.


  Update @since, improve doc

REPOSITORY
  R245 Solid

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28590?vs=81061&id=83112

BRANCH
  arcpatch-D28590_1

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

AFFECTED FILES
  src/solid/devices/backends/fakehw/fakedevice.cpp
  src/solid/devices/backends/fakehw/fakedevice.h
  src/solid/devices/backends/fstab/fstabdevice.cpp
  src/solid/devices/backends/fstab/fstabdevice.h
  src/solid/devices/backends/fstab/fstabmanager.cpp
  src/solid/devices/backends/shared/rootdevice.cpp
  src/solid/devices/backends/shared/rootdevice.h
  src/solid/devices/backends/udev/udevdevice.cpp
  src/solid/devices/backends/udev/udevdevice.h
  src/solid/devices/backends/udev/udevmanager.cpp
  src/solid/devices/backends/udisks2/udisksdevice.cpp
  src/solid/devices/backends/udisks2/udisksdevice.h
  src/solid/devices/backends/udisks2/udisksmanager.cpp
  src/solid/devices/backends/upower/upowerdevice.cpp
  src/solid/devices/backends/upower/upowerdevice.h
  src/solid/devices/backends/upower/upowermanager.cpp
  src/solid/devices/frontend/device.cpp
  src/solid/devices/frontend/device.h
  src/solid/devices/ifaces/device.h

To: meven, #frameworks, bruns, sitter
Cc: ervin, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D28590: Add a QString Solid::Device::displayName, used in Fstab Device for network mounts

[Méven Car]

meven added a subscriber: ervin.
meven added a comment.


  @ervin perhaps you might review this as @bruns seems too busy.

REPOSITORY
  R245 Solid

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

To: meven, #frameworks, bruns, sitter
Cc: ervin, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29050: KRunner fix prepare/teardown signals

[Méven Car]

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


  It is hard to understand why it was wrongly placed here, that makes this hard 
to review and approve.
  But you are getting familiar with KRunner internals, so since it does seem 
benign and you tested it, I think we can merge this.
  
  Please wait a little, to see if others have something to say, before landing.

REPOSITORY
  R308 KRunner

BRANCH
  krunner_signal_bugfix (branched from master)

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

To: alex, meven, ngraham, broulik
Cc: cfeck, kde-frameworks-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, ragreen, michaelh, ZrenBot, ngraham, bruns, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
ahiemstra, mart

D29232: Introduce the Header color set

[Méven Car]

meven added a comment.


  @mart you have a couple of typos left and recommendation to add some doc.

REPOSITORY
  R265 KConfigWidgets

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

To: mart, #vdg, #plasma, cblack, ndavis
Cc: meven, davidre, ndavis, cblack, kde-frameworks-devel, LeGast00n, michaelh, 
ngraham, bruns

D29397: KPreviewJob : Support for DeviceRatioPixel

[Méven Car]

meven added a comment.


  ping dear reviewers

REPOSITORY
  R241 KIO

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

To: meven, dfaure, broulik, #frameworks, ngraham
Cc: elvisangelaccio, kossebau, davidedmundson, kde-frameworks-devel, LeGast00n, 
cblack, michaelh, ngraham, bruns

D29201: KCMUtils: Add option to append service file to list of arguments

[Méven Car]

meven added inline comments.

INLINE COMMENTS

> alex wrote in kpluginselector.cpp:855
> > moduleInfo is part of the ctor here, so the fileName is already available 
> > indirectly.
> 
> Yes, but the KCModuleProxy is just a wrapper class for the KCModule.
> The actual KCModule gets created in kcmoduleloader.cpp line 93+.
> 
> PS: The moduleInfo is also available there, but I don't see any elegant way 
> to make it available to the KCModule other than a property or appending it to 
> the list of arguments.
> 
> >   A good thing would be to avoid having an option for such a niche usage 
> > although legitimate.
> 
> Always open to suggestions 😃

Seems like a property would make sense, after all it is about it, or a ref to 
the KCModuleInfo

REPOSITORY
  R295 KCMUtils

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

To: alex, #plasma, ngraham, meven, broulik, mart
Cc: mart, apol, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

D29381: Thumbnail text: use libmagic to detect encoding

[Méven Car]

meven added a comment.


  I am only half satisfied by the patch.
  Mostly because of libmagic `magic_load` that loads a 5M file each time which 
is not needed to detect encoding.
  
  I would add some tests before landing anyways.

REPOSITORY
  R320 KIO Extras

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

To: meven, #frameworks, sitter, ngraham
Cc: pino, kde-frameworks-devel, kfm-devel, waitquietly, azyx, nikolaik, 
pberestov, iasensio, aprcela, fprice, LeGast00n, cblack, fbampaloukas, alexde, 
Codezela, feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, rdieter, mikesomov

D29381: Thumbnail text: use libmagic to detect encoding

[Méven Car]

meven updated this revision to Diff 83055.
meven marked 5 inline comments as done.
meven added a comment.


  Use QByteArray, find typo, code style and naming

REPOSITORY
  R320 KIO Extras

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29381?vs=82252&id=83055

BRANCH
  arcpatch-D29381

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

AFFECTED FILES
  CMakeLists.txt
  cmake/Findlibmagic.cmake
  thumbnail/CMakeLists.txt
  thumbnail/config-thumbnail.h.cmake
  thumbnail/textcreator.cpp

To: meven, #frameworks, sitter, ngraham
Cc: pino, kde-frameworks-devel, kfm-devel, waitquietly, azyx, nikolaik, 
pberestov, iasensio, aprcela, fprice, LeGast00n, cblack, fbampaloukas, alexde, 
Codezela, feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, rdieter, mikesomov

D29806: Thumbnail folders: scale down sub image when their height is too big

[Méven Car]

meven planned changes to this revision.
meven added a comment.


  Will probably send to gitlab ;)

REPOSITORY
  R320 KIO Extras

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

To: meven, #frameworks, ngraham
Cc: kde-frameworks-devel, kfm-devel, waitquietly, azyx, nikolaik, pberestov, 
iasensio, aprcela, fprice, LeGast00n, cblack, fbampaloukas, alexde, Codezela, 
feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, rdieter, mikesomov

D29397: KPreviewJob : Support for DeviceRatioPixel

[Méven Car]

meven added a comment.


  In D29397#672557 , 
@elvisangelaccio wrote:
  
  > Would it make sense to initialize `devicePixelRatio` to 
`QGuiApplication::devicePixelRatio()` and add an API to change it (if desired) ?
  
  
  It would make KPreviewJob casting a `QCoreApplication::instance` to 
`QGuiApplication::devicePixelRatio` that would be tolerable.
  
  But this would make the feature opt-out : apps could get broken when their 
previews double their size just when changing KF version.
  
  > This way we wouldn't need to call a static method in the `main` of every 
client app (i.e. D29525  wouldn't be 
needed).
  
  This is a single static call for the lifetime of the app, not that hard.
  This makes the feature opt-in, meaning the developer has tested it.

REPOSITORY
  R241 KIO

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

To: meven, dfaure, broulik, #frameworks, ngraham
Cc: elvisangelaccio, kossebau, davidedmundson, kde-frameworks-devel, LeGast00n, 
cblack, michaelh, ngraham, bruns

D29805: Thumbnail djvu: Avoid a crash when djvu is not installed

[Méven Car]

meven updated this revision to Diff 83018.
meven added a comment.


  fix indentation

REPOSITORY
  R320 KIO Extras

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29805?vs=83014&id=83018

BRANCH
  arcpatch-D29805

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

AFFECTED FILES
  thumbnail/djvucreator.cpp

To: meven, #frameworks, broulik, ngraham, pino
Cc: pino, kde-frameworks-devel, kfm-devel, waitquietly, azyx, nikolaik, 
pberestov, iasensio, aprcela, fprice, LeGast00n, cblack, fbampaloukas, alexde, 
Codezela, feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, rdieter, mikesomov

D29805: Thumbnail djvu: Avoid a crash when djvu is not installed

[Méven Car]

meven retitled this revision from "Thumbnail djvu: avoid to exit(1) when it 
should not" to "Thumbnail djvu: Avoid a crash when djvu is not installed".
meven edited the summary of this revision.

REPOSITORY
  R320 KIO Extras

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

To: meven, #frameworks, broulik, ngraham, pino
Cc: pino, kde-frameworks-devel, kfm-devel, waitquietly, azyx, nikolaik, 
pberestov, iasensio, aprcela, fprice, LeGast00n, cblack, fbampaloukas, alexde, 
Codezela, feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, rdieter, mikesomov

D29805: Thumbnail djvu: avoid to exit(1) when it should not

[Méven Car]

meven edited the test plan for this revision.

REPOSITORY
  R320 KIO Extras

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

To: meven, #frameworks, broulik, ngraham, pino
Cc: pino, kde-frameworks-devel, kfm-devel, waitquietly, azyx, nikolaik, 
pberestov, iasensio, aprcela, fprice, LeGast00n, cblack, fbampaloukas, alexde, 
Codezela, feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, rdieter, mikesomov

D29805: Thumbnail djvu: avoid to exit(1) when it should not

[Méven Car]

meven edited the summary of this revision.

REPOSITORY
  R320 KIO Extras

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

To: meven, #frameworks, broulik, ngraham, pino
Cc: pino, kde-frameworks-devel, kfm-devel, waitquietly, azyx, nikolaik, 
pberestov, iasensio, aprcela, fprice, LeGast00n, cblack, fbampaloukas, alexde, 
Codezela, feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, rdieter, mikesomov

D29805: Thumbnail djvu: avoid to exit(1) when it should not

[Méven Car]

meven updated this revision to Diff 83014.
meven added a comment.


  Review feedback

REPOSITORY
  R320 KIO Extras

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29805?vs=83012&id=83014

BRANCH
  arcpatch-D29805

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

AFFECTED FILES
  thumbnail/djvucreator.cpp

To: meven, #frameworks, broulik, ngraham, pino
Cc: pino, kde-frameworks-devel, kfm-devel, waitquietly, azyx, nikolaik, 
pberestov, iasensio, aprcela, fprice, LeGast00n, cblack, fbampaloukas, alexde, 
Codezela, feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, rdieter, mikesomov

D29805: Thumbnail djvu: avoid to exit(1) when it should not

[Méven Car]

meven planned changes to this revision.
meven added a comment.


  Thanks @pino

REPOSITORY
  R320 KIO Extras

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

To: meven, #frameworks, broulik, ngraham, pino
Cc: pino, kde-frameworks-devel, kfm-devel, waitquietly, azyx, nikolaik, 
pberestov, iasensio, aprcela, fprice, LeGast00n, cblack, fbampaloukas, alexde, 
Codezela, feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, rdieter, mikesomov

D29806: Thumbnail folders: scale down sub image when their height is too big

[Méven Car]

meven edited the test plan for this revision.

REPOSITORY
  R320 KIO Extras

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

To: meven, #frameworks, ngraham
Cc: kde-frameworks-devel, kfm-devel, waitquietly, azyx, nikolaik, pberestov, 
iasensio, aprcela, fprice, LeGast00n, cblack, fbampaloukas, alexde, Codezela, 
feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, rdieter, mikesomov

D29806: Thumbnail folders: scale down sub image when their height is too big

[Méven Car]

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

REVISION SUMMARY
  BUG: 419566
  FIXED-IN: 20.08

TEST PLAN
  Before:
  
  After

REPOSITORY
  R320 KIO Extras

BRANCH
  master

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

AFFECTED FILES
  thumbnail/thumbnail.cpp

To: meven, #frameworks, ngraham
Cc: kde-frameworks-devel, kfm-devel, waitquietly, azyx, nikolaik, pberestov, 
iasensio, aprcela, fprice, LeGast00n, cblack, fbampaloukas, alexde, Codezela, 
feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, rdieter, mikesomov

D29805: Thumbnail djvu: avoid to exit(1) when it should not

[Méven Car]

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

REVISION SUMMARY
  A child process exited with 1 as result but it wasn't an error case.
  
  BUG: 420542
  BUG: 420074
  FIXED-IN: 20.08

TEST PLAN
  Previewed a djvu file

REPOSITORY
  R320 KIO Extras

BRANCH
  master

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

AFFECTED FILES
  thumbnail/djvucreator.cpp

To: meven, #frameworks, broulik, ngraham
Cc: kde-frameworks-devel, kfm-devel, waitquietly, azyx, nikolaik, pberestov, 
iasensio, aprcela, fprice, LeGast00n, cblack, fbampaloukas, alexde, Codezela, 
feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, rdieter, mikesomov

D26407: KFileItem: Improve isSlow to not block when a network mount is unresponsive, make SkipMimeTypeFromContent skip only on slow fs

[Méven Car]

meven added inline comments.

INLINE COMMENTS

> kmountpoint.cpp:444
> +// for /dir/link/dir/test will return result for 
> /destLink/dir/test
> +return findByPath(fileinfo.symLinkTarget() + 
> path.mid(cursor));
> +}

Need to pass the flag here.

REPOSITORY
  R241 KIO

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

To: meven, #frameworks, ngraham, broulik, dfaure
Cc: cfeck, anthonyfieroni, kde-frameworks-devel, LeGast00n, cblack, michaelh, 
ngraham, bruns

D29201: KCMUtils: Add option to append service file to list of arguments

[Méven Car]

meven added inline comments.

INLINE COMMENTS

> alex wrote in kpluginselector.cpp:855
> You could add a field to the KCModule class (thats where you ultimately want 
> to access the fileName).
> But then the downside is that you don't have them as a constructor argument.

moduleInfo is part of the ctor here, so the fileName is already available 
indirectly.
A good thing would be to avoid having an option for such a niche usage although 
legitimate.

REPOSITORY
  R295 KCMUtils

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

To: alex, #plasma, ngraham, meven, broulik, mart
Cc: mart, apol, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

D29738: Fix service file specifying 'Run in terminal' giving an error code 100

[Méven Car]

meven added a comment.


  It seems it is causing two failures in kiogui_applicationlauncherjobtest :
  
https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20SUSEQt5.14/105/testReport/junit/projectroot/autotests/kiogui_applicationlauncherjobtest/

REPOSITORY
  R241 KIO

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

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

D29397: KPreviewJob : Support for DeviceRatioPixel

[Méven Car]

meven added a reviewer: ngraham.

REPOSITORY
  R241 KIO

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

To: meven, dfaure, broulik, #frameworks, ngraham
Cc: kossebau, davidedmundson, kde-frameworks-devel, LeGast00n, cblack, 
michaelh, ngraham, bruns

D29397: KPreviewJob : Support for DeviceRatioPixel

[Méven Car]

meven added a comment.


  ping reviewers

INLINE COMMENTS

> previewjob.cpp:754
>  thumb.setText(QStringLiteral("Thumb::Mimetype"), 
> currentItem.item.mimetype());
> +thumb.setText(QStringLiteral("Thumb::DevicePixelRatio"), 
> QString::number(thumb.devicePixelRatio()));
>  QString thumbnailerVersion = 
> currentItem.plugin->property(QStringLiteral("ThumbnailerVersion"), 
> QVariant::String).toString();

Maybe add it optionally, when different from 1

REPOSITORY
  R241 KIO

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

To: meven, dfaure, broulik, #frameworks
Cc: kossebau, davidedmundson, kde-frameworks-devel, LeGast00n, cblack, 
michaelh, ngraham, bruns

D29406: Add X-KDE-Original-Executable to Applications properties

[Méven Car]

meven removed a dependent revision: D29407: ScreenshotEffect: Use Service 
Property to authorize screenshot without confirmation.

REPOSITORY
  R309 KService

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

To: meven, #frameworks, davidedmundson, apol, bport
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29774: Add X-KDE-DBUS-Restricted-Interfaces to Application desktop entry fields

[Méven Car]

meven added a dependent revision: D29407: ScreenshotEffect: Use Service 
Property to authorize screenshot without confirmation.

REPOSITORY
  R309 KService

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

To: meven, davidedmundson, zzag, #frameworks, #kwin
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29774: Add X-KDE-DBUS-Restricted-Interfaces to Application desktop entry fields

[Méven Car]

meven created this revision.
meven added reviewers: davidedmundson, zzag, Frameworks, KWin.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
meven requested review of this revision.

REVISION SUMMARY
  This added field to Application desktop entries allows to declare an access 
request to a DBUS interface.
  
  Needed for D29407 

REPOSITORY
  R309 KService

BRANCH
  master

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

AFFECTED FILES
  src/services/application.desktop

To: meven, davidedmundson, zzag, #frameworks, #kwin
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29767: CopyJob: Check if destination dir is a symlink

[Méven Car]

meven updated this revision to Diff 82920.
meven marked 3 inline comments as done.
meven added a comment.


  review feedback, cleanups

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29767?vs=82902&id=82920

BRANCH
  arcpatch-D29767

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

AFFECTED FILES
  autotests/jobtest.cpp
  autotests/jobtest.h
  src/core/copyjob.cpp

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

D29767: CopyJob: Check if destination dir is a symlink

[Méven Car]

This revision was automatically updated to reflect the committed changes.
Closed by commit R241:8769b6360d87: CopyJob: Check if destination dir is a 
symlink (authored by meven).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29767?vs=82920&id=82921

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

AFFECTED FILES
  autotests/jobtest.cpp
  autotests/jobtest.h
  src/core/copyjob.cpp

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

D29767: CopyJob: Check if destination dir is a symlink

[Méven Car]

meven updated this revision to Diff 82902.
meven added a comment.


  Remove unrelated change

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29767?vs=82901&id=82902

BRANCH
  master

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

AFFECTED FILES
  autotests/jobtest.cpp
  autotests/jobtest.h
  src/core/copyjob.cpp

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

D28902: CopyJob: when stat'ing the dest, use StatBasic.

[Méven Car]

meven added a comment.


  Fix at D29767 

REPOSITORY
  R241 KIO

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

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

D29767: CopyJob: Check if destination dir is a symlink

[Méven Car]

meven created this revision.
meven added reviewers: Frameworks, dfaure, ngraham.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
meven requested review of this revision.

REVISION SUMMARY
  BUG: 421213
  FIXED-IN: 5.71

TEST PLAN
  ctest

REPOSITORY
  R241 KIO

BRANCH
  master

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

AFFECTED FILES
  autotests/jobtest.cpp
  autotests/jobtest.h
  src/core/copyjob.cpp
  src/core/kioglobal_p.h

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

D28902: CopyJob: when stat'ing the dest, use StatBasic.

[Méven Car]

meven added a comment.


  Fix and test coming

REPOSITORY
  R241 KIO

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

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

D28902: CopyJob: when stat'ing the dest, use StatBasic.

[Méven Car]

meven added a comment.


  In D28902#671260 , @ngraham wrote:
  
  > @dfaure git bisect says this caused 
https://bugs.kde.org/show_bug.cgi?id=421213.
  >
  > After fixing, maybe we should get a test for that use case?
  
  
  I see the issue, because we don't resolve the symlink here, we don't figure 
out dest is a folder and hence copyjob acts as if moving file to dest file, 
instead of to dest dir.

REPOSITORY
  R241 KIO

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

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

D29634: sftp: break large writes into multiple requests

[Méven Car]

meven added a comment.


  In D29634#670485 , @feverfew wrote:
  
  > In D29634#670419 , @meven wrote:
  >
  > > In D29634#670377 , @ngraham 
wrote:
  > >
  > > > Nice work.
  > > >
  > > > In D29634#670159 , @feverfew 
wrote:
  > > >
  > > > > I imagine something similar should be done for FileJob::write?
  > > >
  > > >
  > > > Yeah.
  > >
  > >
  > > I guess you meant `FileProtocol::write`.
  > >  There is no need there, it uses `QIODevice::write` directly.
  >
  >
  > Sorry, I meant `kio_sftp`'s implementation of this: 
https://api.kde.org/frameworks/kio/html/classKIO_1_1FileJob.html#a481871536fb9471ccb64929792f31165
  
  
  I believe this is taken care of this patch in `SFTPInternal::write`

REPOSITORY
  R320 KIO Extras

BRANCH
  release/20.04

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

To: sitter, ngraham, meven, feverfew
Cc: meven, feverfew, kde-frameworks-devel, kfm-devel, waitquietly, azyx, 
nikolaik, pberestov, iasensio, aprcela, fprice, LeGast00n, cblack, 
fbampaloukas, alexde, Codezela, michaelh, spoorun, navarromorales, firef, 
ngraham, andrebarros, bruns, emmanuelp, rdieter, mikesomov

D29634: sftp: break large writes into multiple requests

[Méven Car]

meven added a comment.


  In D29634#670377 , @ngraham wrote:
  
  > Nice work.
  >
  > In D29634#670159 , @feverfew 
wrote:
  >
  > > I imagine something similar should be done for FileJob::write?
  >
  >
  > Yeah.
  
  
  I guess you meant `FileProtocol::write`.
  There is no need there, it uses `QIODevice::write` directly.

REPOSITORY
  R320 KIO Extras

BRANCH
  release/20.04

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

To: sitter, ngraham, meven, feverfew
Cc: meven, feverfew, kde-frameworks-devel, kfm-devel, waitquietly, azyx, 
nikolaik, pberestov, iasensio, aprcela, fprice, LeGast00n, cblack, 
fbampaloukas, alexde, Codezela, michaelh, spoorun, navarromorales, firef, 
ngraham, andrebarros, bruns, emmanuelp, rdieter, mikesomov

D29526: Thumbnails: make thumbnail generation dpr-aware

[Méven Car]

meven added inline comments.

INLINE COMMENTS

> sitter wrote in thumbnail.cpp:774
> Var naming is a bit inconsistent across the code base now. In the 
> implementations there are maxWidth/H that are device-adjusted but in here 
> they are not. Not a huge concern, just noticed.

Will take care

> thumbnail.cpp:791
>  // for the same directory and sequence-item
>  qsrand(qHash(filePath));
>  

Not used can be removed.

> thumbnail.cpp:794
>  // Apply fake smooth scaling, as seen on several blogs
> -if (subThumbnail.width() > width * 4 || subThumbnail.height() > height * 
> 4) {
> -subThumbnail = subThumbnail.scaled(width*4, height*4, 
> Qt::KeepAspectRatio, Qt::FastTransformation);
> +if (subThumbnail.width() > width * 4 * subThumbnail.devicePixelRatio() 
> || subThumbnail.height() > height * 4 * subThumbnail.devicePixelRatio()) {
> +subThumbnail = subThumbnail.scaled(width * 4 * 
> subThumbnail.devicePixelRatio(), height * 4 * 
> subThumbnail.devicePixelRatio(), Qt::KeepAspectRatio, Qt::FastTransformation);

Because of line above `scaleDownImage` can never happen.
Can probably be removed.

REPOSITORY
  R320 KIO Extras

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

To: meven, #frameworks, dfaure, broulik, sitter, ngraham
Cc: kde-frameworks-devel, kfm-devel, waitquietly, azyx, nikolaik, pberestov, 
iasensio, aprcela, fprice, LeGast00n, cblack, fbampaloukas, alexde, Codezela, 
feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, rdieter, mikesomov

D29634: sftp: break large writes into multiple requests

[Méven Car]

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

REPOSITORY
  R320 KIO Extras

BRANCH
  release/20.04

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

To: sitter, ngraham, meven
Cc: meven, feverfew, kde-frameworks-devel, kfm-devel, waitquietly, azyx, 
nikolaik, pberestov, iasensio, aprcela, fprice, LeGast00n, cblack, 
fbampaloukas, alexde, Codezela, michaelh, spoorun, navarromorales, firef, 
ngraham, andrebarros, bruns, emmanuelp, rdieter, mikesomov

D29708: Introduce EXCLUDE_DEPRECATED_BEFORE_AND_AT

[Méven Car]

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


  Great stuff, this much less cleanup will be needed for KF6 !

INLINE COMMENTS

> global.cpp:99
>  
> +#if KIOCORE_BUILD_DEPRECATED_SINCE(3, 4)
>  KIOCORE_EXPORT QTime KIO::calculateRemaining(KIO::filesize_t totalSize, 
> KIO::filesize_t processedSize, KIO::filesize_t speed)

Pretty old deprecation ^^

REPOSITORY
  R241 KIO

BRANCH
  excludedeprecated

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

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

D29707: Remove deprecation tag from SlaveBase::config() for now

[Méven Car]

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

REPOSITORY
  R241 KIO

BRANCH
  removedeprecatetagfromslavebaseconfig

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

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

D23523: [SlaveBase] Use QMap instead of KConfig to store ioslave config

[Méven Car]

meven added inline comments.

INLINE COMMENTS

> kossebau wrote in slavebase.h:355
> Given there are still a few usages of config() left which seem not easily 
> replaceable, it would be better to remove the deprecation tag for the 
> compiler, to not have false warnings on those places (see e.g. http.cpp for 
> certain usages still).
> A deprecation tag should be only set if there is a full migration path 
> available ideally, otherwise people will run into warnings they cannot do 
> something about.

Agreed, perhaps update the documentation slightly

REPOSITORY
  R241 KIO

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

To: meven, davidedmundson, dfaure, #frameworks
Cc: kossebau, broulik, kde-frameworks-devel, LeGast00n, cblack, michaelh, 
ngraham, bruns

D29201: KCMUtils: Add option to append service file to list of arguments

[Méven Car]

meven added inline comments.

INLINE COMMENTS

> kpluginselector.cpp:855
> +}
> +KCModuleProxy *currentModuleProxy = new 
> KCModuleProxy(moduleInfo, moduleProxyParentWidget, arguments);
>  if (currentModuleProxy->realModule()) {

Adding a fileName field to KCModuleProxy would make more sense to me, and do it 
by default.
Plus KCModuleProxy has already access to the fileName since it receives 
moduleInfo. 
But I am not a specialist.

REPOSITORY
  R295 KCMUtils

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

To: alex, #plasma, ngraham, meven, broulik, mart
Cc: mart, apol, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns