KDEReview for Kontrast
carl at carlschwan.eu
Mon Aug 3 03:14:34 BST 2020
‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
Le dimanche, août 2, 2020 3:36 PM, Nicolas Fella <nicolas.fella at gmx.de> a écrit :
> Hi Carl,
> a couple of nitpicks, otherwise looks neat.
> - your CMakeLists.txt does not specify a minimum Qt/KF5 version. Also
> ECM 0.0.8 is likely quite old and a bit optimistic
> - Setting CMAKE_CXX_STANDARD to 11 is implicitly done by ECM, no need to
> do that manually. It also contradicts the
> target_compile_features(kontrast PRIVATE cxx_std_17) you set later
> - You can save yourself the explicit call to qt5_add_resources by adding
> resources.qrc to the add_executable call. This is called AUTORCC in
> cmake and ECM enables it by default
It is probably worth changing this in the default Kirigami template in
KAppTemplete since this is where I took the code. Same for the CMAKE_CXX_STANDARD
declaration and the ECM 0.0.8.
> - Instead of using QScopedPointer<Kontrast> you should be able to just
> put Kontrast on the stack
> - consider setting "isMenu: true" on your global drawer, that turns it
> into a hamburger menu on the desktop, which is more appropriate than a
Thanks for the feedback, I think I have now fixed all the things you found.
> On 30.07.20 11:16, Carl Schwan wrote:
> > Hi,
> > I would like to move Kontrast, a contrast checker application, to KDEReview. Kontrast can check if two colors pass the WCAG 2.0 specification and save some user's favorite color combinations.
> > Some screenshots of the application and a design review from the VSG is available here: https://invent.kde.org/accessibility/kontrast/-/issues/1
> > From a code point of view, the application is very simple, but I still would appreciate a general code review on it (it's my first Qt app written from scratch). The code is available here: https://invent.kde.org/accessibility/kontrast
> > I don't plan to add new features and would like after the KDEReview, to release a first version of the application, and then move it to the release service so that the application gets regularly translations improvement.
> > Thanks
> > Carl
More information about the kde-core-devel