Re: Keysmith in kdereview
Albert Astals Cid wrote: > I think it'd be good if you used a QVarLengthArray instead of "char > code[m_pinLength];" Sadly, QVarLengthArray is much less efficient, because it cannot do variable-size stack allocation, only fixed-size stack allocation (wasting stack space) and heap allocation for anything that does not fit (wasting the entire stack space and slowing down the application). But unfortunately, it is the only thing that will compile on M$VC and other stubborn compilers that refuse to implement VLAs and whose developers have successfully sabotaged all attempts to bring this standardized C99 feature into the C++ standard (and even got C11 to make it optional for C too). Kevin Kofler
Re: Keysmith in kdereview
El dimecres, 18 de desembre de 2019, a les 15:50:36 CET, Bhushan Shah va escriure: > Hello everyone! > > Keysmith (https://invent.kde.org/kde/keysmith) has been moved to > kdereview. > > Keysmith is a Two-factor code generator for Plasma Mobile and Desktop > and is using oath-toolkit for this purpose. User interface is written in > the kirigami. I think it'd be good if you used a QVarLengthArray instead of "char code[m_pinLength];" Cheers, Albert > > Thanks! > >
Re: Keysmith in kdereview
Hi Bhushan, Am Mittwoch, 18. Dezember 2019, 15:50:36 CET schrieb Bhushan Shah: > Hello everyone! > > Keysmith (https://invent.kde.org/kde/keysmith) has been moved to > kdereview. > > Keysmith is a Two-factor code generator for Plasma Mobile and Desktop > and is using oath-toolkit for this purpose. User interface is written in > the kirigami. Did some usual-nitpick-CMake-code cleanup commit already (things which also apply to other new Plasma repos, someone might want to brush over their CMakeLists.txt as well, using that commit as reference). Other things noticed on superficial look: * UI not translated, i18n support setup missing completely * uses own "ENABLE_TESTING", not "BUILD_TESTING" flag from KDECompilerSettings proposed: + switch flag use to BUILD_TESTING - remove option(ENABLE_TESTING "Enable tests" ON) - remove enable_testing() (done by KDECompilerSettings) * you might want to check if KF 5.37 has minimum version of Kirigami features used, otherwise needs bumping Also surprised to see org.kde namespace added to the binary executable name, is that a new xdg recommendation? No clue about purpose of app otherwise, so no usage tested besides starting :) Cheers Friedrich
Keysmith in kdereview
Hello everyone! Keysmith (https://invent.kde.org/kde/keysmith) has been moved to kdereview. Keysmith is a Two-factor code generator for Plasma Mobile and Desktop and is using oath-toolkit for this purpose. User interface is written in the kirigami. Thanks! -- Bhushan Shah http://blog.bshah.in IRC Nick : bshah on Freenode GPG key fingerprint : 0AAC 775B B643 7A8D 9AF7 A3AC FE07 8411 7FBC E11D signature.asc Description: PGP signature
