KDEReview for Kontrast

[Nicolas Fella]

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

- 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


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