KDEReview for Kontrast

[Carl Schwan]

Carl Schwan
https://carlschwan.eu

‐‐‐‐‐‐‐ 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
>     drawer

Thanks for the feedback, I think I have now fixed all the things you found.
>
>     Cheers
>
>     Nico
>
>     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