On Saturday, 2 April 2022 11:28:11 CEST Gilles Caulier wrote: > Hi, > > Just a question about Qt6 support in KSane* API. Do you plan this kind > of support in a near future? As i can see libksane is not yet ported. > > Best regards > > Gilles Caulier
Hi, I turned on Qt6 CI for KSANECore and it works just fine. I don't know in what state the rest of libksane is in for Qt6. I will have a look when libksane uses KSANECore, so hopefully this will be sorted out during the KDE Gear 22.08 release cycle. Best regards, Alex > Le sam. 2 avr. 2022 à 11:26, Alexander Stippich <a.stipp...@gmx.net> a écrit : > > On Sunday, 27 March 2022 21:28:38 CEST Harald Sitter wrote: > > > On Sun, Mar 27, 2022 at 6:29 PM Alexander Stippich <a.stipp...@gmx.net> > > > > wrote: > > > > Hello everyone, > > > > > > > > KSANECore is now in KDE review. Kåre and I mentioned it in previous emails > > > > before, but as a short summary: > > > > KSANECore is a Qt interface to the SANE scanner library. It is stripped > > > > out of > > > > > > the KSaneWidget of libksane without any QWidget dependency. It is > > > > currently > > > > > > located inside the libksane repository as KSaneCore and basically just > > > > copied > > > > > > into the new repository. > > > > > > > > Due to breaking API anyway, the code was cleaned up, better named as well > > > > as > > > > > > smaller API fixes made on top. Also, KSANECore is fully REUSE compliant. > > > > KSaneWidget of libksane will remain ABI compatible. > > > > > > > > I don't know if it is strictly required to move the new repo with already > > > > existing code through KDE review, but I guessed it is better to be on the > > > > safe > > > > > > side :) > > > > > > > > The plan is to switch libksane and Skanpage over to the new library during > > > > the > > > > > > KDE Gear 22.08 release cycle. The adaptions are located at > > > > https://invent.kde.org/astippich/skanpage/-/commits/ksanecore > > > > and > > > > https://invent.kde.org/astippich/libksane/-/commits/ksanecoreSplit > > > > > > amazing! > > > > > > for everyone's convenience here's the url to the lib ;) > > > https://invent.kde.org/libraries/ksanecore > > > > Thanks! Could have thought about this myself... > > > > > some comments from scrolling through the code: > > > > > > you may want to reconsider how stringEnumTranslation works > > > https://github.com/KDE/clazy/blob/master/docs/checks/README-non-pod-global-static.md > > > either use q_global_static, or - IMHO neater - move it into the > > > function loadDeviceOptions as function local static since we don't > > > need it outside that function anyway. > > > > > > CoreInterface, CoreInterfacePrivate, InternalOption, ScanThread and > > > CoreOption should have their constructors marked explicit > > > > > > the typedefs in coreoption.h are super old school maybe modernize them? ;) > > > > > > some of the API documentation still refers to libksane, should that be > > > > changed? > > > > All done. > > > > > on a similar note, you still use the KSane namespace and include > > > guards. It may make sense to rename them KSaneCore and KSANECORE > > > respectively? for consistency if nothing else > > > > KSane was chosen deliberately here. The plan is to have a KSane::Core... and > > the KSane::Widget in the future. libksane would become KSaneWidget later > > during the Qt 6 transition, to keep ABI compatibility for now. > > > > > if you havent considered this: you might want to use the clang-format > > > rules from ECM to join the common formatting we have (and apply that > > > via commit hooks) > > > > Not done yet, but I will look into it. > > > > > I would argue that reloadDevicesList and getOption(const OptionName > > > optionEnum); should have their const dropped. the const there doesn't > > > impact the signature and as such is confusing from an API POV > > > > > > on a matter of less code noise I would use std::make_unique when > > > creating new unique ptrs instead of manually passing a raw pointer to > > > the ctor. > > > > > > in CoreInterfacePrivate m_batchMode + Delay are uninitialized in the > > > ctor. please initialize them nullptr > > > > > > since you have Qt6 ifdefs you may want to enable qt6 CI as well > > > > > > the shebang line in your Messages.sh appears to have lost its position > > > and is no longer at the top of the file > > > > All fixed. > > > > > you appear to have an autotests dir and cmakelists but no actual tests? :O > > > > That is reserved for the future when I find the time to actually write the > > tests :) > > > > Thanks for the review! > > > > Best regards, > > Alex
