Subject: [bug#58125] [PATCH] gnu: Add nm-tray

From: Maxime Devos <[email-masked]>
Date: Thu, 29 Sep 2022 18:29:12 +0200

> Subject: [PATCH] gnu: Add nm-tray * gnu/packages/networking.scm (nm-tray): New
>  variable.

Except for the initial line(?), the commit message is for tor the 
Subject line (-:.

On 27-09-2022 22:49, Pavel Shlyak wrote:

 > + ;;; Copyright © 2020-2022 PantherX OS Team <[email-masked]>

Please write the actual copyright holder (likely the relevant members of 
this team or perhaps their employer, depending on how things are set up) 
-- AFAIK, "PantherX OS Team" is a not some legal entity that can hold 
copyright.

In both cases, I think we should investigate whether something like 
<https://www.samba.org/samba/devel/copyright-policy.html> should be done 
in Guix first.

 > +++ b/gnu/packages/networking.scm
 > +  #:use-module (gnu packages kde-frameworks)

I don't think this module should unconditionally import that, to help 
with keeping "guix ... package" fast when the kde-frameworks isn't 
actually used.

I have tried out some tricks in <https://issues.guix.gnu.org/54539> -- 
while there doesn't seem to have been any conclusion on what's the 
'proper' trick, there are some options -- see, e.g., how 
https://issues.guix.gnu.org/57861 uses module-ref.

> +   (synopsis "NetworkManager front end with information icon residing in system tray")
> +   (description "nm-tray is a network connection management tool (NetworkManager front end)
> +with information icon residing in system tray (like e.g. nm-applet).")

Could you add some information, more specifically something to 
distinguish nm-applet from nm-applet?  Currently, it doesn't give any 
information that helps the user with choosing between nm-tray nad nm-applet.

> +	   (recursive? #t)))

Why? AFAICT, there are no git submodules.

Looking through the source code, it appears to use icons.  Can you test 
that icons are displayed properly in a pure environment ("guix shell 
--pure nm-tray -- ...")?  Sometimes graphical software doesn't find its 
icons without some help.

Looking at CMakeLists.txt, it comes with optional modem manager support, 
for which you seem to have added modemmanager-qt, but from reading 
CMakeLists.txt, it doesn't seem to be enabled by default, could you set 
WITH_MODEMMANAGER_SUPPORT?

Greetings,
Maxime.