D28909: smb: port to Result system to force serialization of error/finish condition

David Faure noreply at phabricator.kde.org
Sat Apr 25 13:41:09 BST 2020


dfaure added inline comments.

INLINE COMMENTS

> sitter wrote in kio_smb.h:96
> Sure, if you think it's solid enough from an API POV.
> 
> I was thinking that we should amend the slavebase API for KF6 in general. Instead of having error/finished/opened all functions on an API level should return a Result and the slave loop would emit the relevant signal based on the Result. IOW: what currently happens in the derived SlaveBases actually ought to be KIO-internal.
> 
> That would then also allow us to get rid of the two-class split again. And the "fronting" class is actually a much bigger concern than Result to me. The call finalization logic is 100% code copy and so very easy to get wrong (e.g. sftp's special() not finishing when in fact it should).

Excellent idea, but why wait for KF6?

We could implement a subclass of SlaveBase, in KIO, let's call it SlaveBaseV2, which

- Reimplements all the virtual methods from SlaveBase, in a private section
- Defines a new of virtual methods, called differently somehow
- Implements the signal emission based on the return value of those new virtual methods.

This allows for incremental porting of the slaves, instead of a big KF6 breakage.
And the V2 class can be developed in a single slave first, before having to freeze its API.

REPOSITORY
  R320 KIO Extras

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

To: sitter, dfaure
Cc: meven, kde-frameworks-devel, kfm-devel, azyx, nikolaik, pberestov, iasensio, aprcela, fprice, LeGast00n, cblack, fbampaloukas, alexde, Codezela, feverfew, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, rdieter, mikesomov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200425/5c2df247/attachment.html>


More information about the Kde-frameworks-devel mailing list