Hi everyone I have touched upon any concerns people have had with the ioslave in my previous email. If no one has any objections, I would like to ship this to KDE Extra Modules. More specifically, it will be suited for kde-extragear-utils, if that division is still relevant in KDE extragear. The sysadmin task for the same can be found here: https://phabricator.kde.org/T6285
On Sat, May 27, 2017 at 3:43 PM, Arnav Dhamija <arnav.dham...@gmail.com> wrote: > First off, thanks for all the reviews! > >> >> We simulate a "virtual://" protocol[*] which can contain virtual >> folders containing references to real files and folders. > > > I did not know that such a feature exists! It seems like this feature is > Krusader specific without tapping into KIO as such? > >> It would be great if we can get rid of this "Krusader only" solution and >> replace it with >> your *real* KIO protocol. Some questions: Is it persistent (between >> reboots)? And does it >> support general file URLs (e.g. ftp://localhost/path/to/file.txt) or only >> local >> "file://"-files? > > > The list of files is not persistent across reboots. That idea was scrapped > fairly early on during the development of this project. It could probably be > implemented using xattr if such a feature is needed. I haven't tested much > on other protocols, but it does have problems copying from some protocols > such as mtp. > > That said, being a kioslave, it works well with Dolphin, the KDE Folder view > Plasmoid, and Konqueror for file operations. > >> * ../src/iodaemon/stashnotifier.cpp:183:9: warning: variable 'fileType' is >> uninitialized when used here >> This should probably be fixed. >> >> * You are linking to KI18n but you are not using i18n() calls in your >> code. Have a look at [1]. >> >> * The dbus adaptor could probably use build-time generation via cmake, >> rather than being committed to the git repo. If you need an example look at >> the CMakeLists.txt in kio/src/kioexec (qt5_add_dbus_adaptor and friends). > > > Thanks, I have fixed the first two issues. My response to the last issue is > given below. > >> Files that contain autogenerated code should ideally not be "edited". >> >> Do you remember why editing was needed? > > >> Not exactly, Arnav will know that. But it's not really too big a >> problem for a project this size. In fact I do the same thing in >> Spectacle. > > > I believe we needed to make some specific changes to the autogenerated XML > at some point in the project. These changes were later scrapped, so it uses > the directly autogenerated code with no modifications. As Boudhayan said, > the justification for not changing it to be invoked on compilation was for > simplicity and that the D-Bus code was rarely updated throughout the > development of the project. > >> Maintainers will leave and we will inherit the code, so there will be >> noone to >> ask. > > > I will be available to maintain this code when it gets released in the KDE > software ecosystem. > > > On Sat, May 27, 2017 at 2:53 PM, Albert Astals Cid <aa...@kde.org> wrote: >> >> El dissabte, 27 de maig de 2017, a les 0:29:51 CEST, Boudhayan Gupta va >> escriure: >> > Hi, >> > >> > On 27 May 2017 at 00:20, Albert Astals Cid <aa...@kde.org> wrote: >> > > El divendres, 26 de maig de 2017, a les 23:48:18 CEST, Boudhayan Gupta >> > > va >> > > >> > > escriure: >> > >> Hi, >> > >> >> > >> On 26 May 2017 at 20:31, Elvis Angelaccio <elvis.angelac...@kde.org> >> wrote: >> > >> > * The dbus adaptor could probably use build-time generation via >> > >> > cmake, >> > >> > rather than being committed to the git repo. If you need an example >> > >> > look >> > >> > at >> > >> > the CMakeLists.txt in kio/src/kioexec (qt5_add_dbus_adaptor and >> > >> > friends). >> > >> >> > >> IIRC we did that first, but then figured we needed to edit the >> > >> generated code (I was the mentor for this project). >> > > >> > > That seems dangerous, what if sometime in the future you need to >> > > regenerate >> > > the adaptor because you add new functions or something? >> > > >> > > Files that contain autogenerated code should ideally not be "edited". >> > > >> > > Do you remember why editing was needed? >> > >> > Not exactly, Arnav will know that. But it's not really too big a >> > problem for a project this size. In fact I do the same thing in >> > Spectacle. >> > >> > Granted this isn't best practice, but the effort required to do it >> > right (whatever way that might be) was inordinately large for the >> > scope of the project. At some point someone who's trying to patch this >> > part of the code might get stuck, but figuring out that generated code >> > was edited isn't difficult at all, as neither is asking the maintainer >> > how the code works. >> >> Maintainers will leave and we will inherit the code, so there will be >> noone to >> ask. >> >> If you need to do some changes to generated code either document it or >> just >> don't do changes in the generated code. >> >> Cheers, >> Albert >> >> > >> > Thanks, >> > Boudhayan >> > >> > > Cheers, >> > > >> > > Albert >> > > >> > >> > Cheers, >> > >> > Elvis >> > >> > >> > >> > [1]: https://api.kde.org/frameworks/ki18n/html/prg_guide.html >> > >> >> > >> Freundliche Grüße >> > >> Boudhayan Gupta >> > >> KDE e.V. - Sysadmin and Community Working Groups >> > >> +49 151 71032970 >> >> > > > > -- > arnav dhamija -- arnav dhamija