[bug#39619] [v2] Re: bug#39619: Acknowledgement ([PATCH 0/4] Add nheko matrix client)

[Nicolas Goaziou <mail@nicolasgoaziou.fr>]

Hello,

Nicolò Balzarotti <anothersms@gmail.com> writes:

> I just noticed that nlohmann-json-cpp is deprecated for json-modern-cxx,
> fixed it in the two patches that were using it.

Thank you for the patches.

Unfortunately, I cannot build nheko because of a missing lmdbxx input.

Some comments follow.

> +           (lambda _
> +             (substitute* "CMakeLists.txt"
> +               (("add_test\\(BasicConnectivity") "# add_test")
> +               (("add_test\\(ClientAPI") "# add_test")
> +               (("add_test\\(MediaAPI") "# add_test")
> +               (("add_test\\(Encryption") "# add_test"))

Nitpick: I suggest to use a single regexp for these.

> +    (inputs
> +     `(("boost" ,boost)
> +       ("libolm" ,libolm)
> +       ("libsodium" ,libsodium)
> +       ("openssl" ,openssl)
> +       ("json-modern-cxx" ,json-modern-cxx)
> +       ("spdlog" ,spdlog)
> +       ("zlib" ,zlib)))

Could you re-order inputs alphabetically?

> +    (description "@code{mtxclient} is a C++ library that implements client API
> +for the Matrix protocol.  It's built on to of @code{Boost.Asio}.")

Nitpick: "It's" -> "It is".
> +    (license license:expat)))
> +
>  (define-public quaternion
>    (package
>      (name "quaternion")
> @@ -1795,8 +1849,8 @@ QMatrixClient project.")
>       (origin
>         (method git-fetch)
>         (uri (git-reference
> -              (url "https://github.com/QMatrixClient/Quaternion")
> -              (commit version)))
> +             (url "https://github.com/QMatrixClient/Quaternion")
> +             (commit version)))

This change is unrelated to the patch. Could you remove it?

> +    (inputs
> +     `(("boost" ,boost)
> +       ("cmark" ,cmark)
> +       ("libolm" ,libolm)
> +       ("lmdb" ,lmdb)
> +       ("lmdbxx" ,lmdbxx)

What is that?
> +       ("mtxclient" ,mtxclient)
> +       ("openssl" ,openssl)
> +       ("json-modern-cxx" ,json-modern-cxx)
> +       ("qtbase" ,qtbase)
> +       ("qtsvg" ,qtsvg)
> +       ("qtmultimedia" ,qtmultimedia)
> +       ("spdlog" ,spdlog)
> +       ("tweeny" ,tweeny)
> +       ("zlib" ,zlib)))
> +    (native-inputs
> +     `(("pkg-config" ,pkg-config)
> +       ("qtlinguist" ,qttools)))

Isn't it a bit confusing?

> +    (build-system qt-build-system)

Nitpick: usually, build-system is above inputs and arguments.

> +    (home-page "https://github.com/Nheko-Reborn/nheko")
> +    (synopsis "Desktop client for Matrix using Qt and C++14")
> +    (description "@code{Nheko} want to provide a native desktop app for the
> +Matrix protocol that feels more like a mainstream chat app and less like an IRC
> +client.

"that feels more..." sounds link marketing buzz. Maybe we could remove it.

> +Most of the features you would expect from a chat application are missing right
> +now but we are getting close to a more feature complete client.

I'm not sure this part is warranted either.

> Specifically
> +there is support for:
> +@itemize
> +@item E2E encryption (text messages only: attachments are currently sent unencrypted).
> +@item User registration.
> +@item Creating, joining & leaving rooms.
> +@item Sending & receiving invites.
> +@item Sending & receiving files and emoji.
> +@item Typing notifications.
> +@item Username auto-completion.
> +@item Message & mention notifications.
> +@item Redacting messages.
> +@item Read receipts.
> +@item Basic communities support.
> +@item Room switcher (@key{ctrl-K}).
> +@item Light, Dark & System themes.
> +@end itemize\n")

No need for the final newline.

Regards,

-- 
Nicolas Goaziou