[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-core-devel
Subject: Re: MauiKit and Index review
From: Carl Schwan <carl () carlschwan ! eu>
Date: 2020-12-01 13:37:51
Message-ID: aD-ZyGWIZ-6EFCo2jOWKbtaXv6PWb3v_IuemY3OLxnKX6z6dNNdvDMrq__cYk7Yrtj7FWiIqKnGHMK6hf2RWRjwqGTq9XkfrW7WHJIin1TQ= () carlschwan ! eu
[Download RAW message or body]
Le lundi, septembre 28, 2020 9:02 AM, Camilo Higuita Rodriguez \
<chiguitar@unal.edu.co> a écrit :
Hi, some feedback:
Index looks visually really nice, great job on that front :)
In term of technical review:
* There are tons of clazy warning in your codebase: I fixed some of them in [1] but
there are more of them. I would encourage you to look into setting up clazy, it
provides tons of helpful advice.
* https://invent.kde.org/maui/mauikit/-/blob/v1.2/src/platforms/linux/kdeconnect.cpp#L41
This won't work if I set LANGUAGE=fr_FR as my environment variable. Also I don't \
think it is a good idea to parse the command line output like this. You should \
probably ask the kdeconnect team if they can add machine-readable interface or a \
dbus API for your usecase.
* https://invent.kde.org/maui/mauikit/-/blob/v1.2/src/utils/syncing/syncing.cpp#L198
This strings should be translated if they are visible to the user.
* You are using qDebug in mauikit, you should probably use QLoggingCategory instead.
Cheers,
Carl
[1]: https://invent.kde.org/maui/mauikit/-/merge_requests/25/diffs
> Hi,
> For the next stable release, we would like to go through KDE review first.
>
> To start I want to submit MauiKit and Index for review, and later on the other \
> apps. https://invent.kde.org/maui/mauikit
> https://invent.kde.org/maui/index
>
> The changes we have made to get to this review are all in the development branch.
>
> I will be available to perform any needed fixes and answer any questions since I'm \
> the main developer and maintainer.
> Greetings,
> Camilo Higuita
>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic