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

[Maxime Devos <maximedevos@telenet.be>]

[-- Attachment #1.1.1: Type: text/plain, Size: 5843 bytes --]

On 05-10-2022 16:46, Pavel Shlyak wrote:
>> Except for the initial line(?), the commit message is for tor the Subject line (-:.
> 
> I didn’t get you

Oops, I meant ‘Except for the initial line(?), the commit message is for 
tor the Subject line (-:.’, no Tor involved.  Your patch looked like this:

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


>  > 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.
> 
> Done

Looks ok, though I think it's best to get some formal confirmation from 
the employer (see: the Samba link).

> 
>> 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 didn’t get that, too

I don't have any additional explanation to give and I gave you an 
example with the reference to 57861.

>> 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.
> 
> Done

 > Unlike nm-applet, that is part of gnome, this app is desktop-unaware.

IIUC, standard capitalisation is GNOME, not gnome.

Also,

   (1) being unaware of desktops doesn't sound like an upside to me.
       Mentioning downsides too is good, but currently nm-tray sounds
       strictly worse than than nm-applet.
   (2) doing a cursory look through nm-applet, it appears to be desktop-
       unaware too -- yes, it's part of GNOME, but nevertheless it
       doesn't seem to care which desktop environment it is run in.

       For example, it looks like it runs just fine in KDE:
 
<https://fedoraproject.org/wiki/KDE/Using_nm_applet_instead_of_knetworkmanager>

If you meant that it has a Qt-ish interface instead of a GTK-ish 
interface (which can be good for consistency) you could write something 
like 'Unlike nm-applet, this application uses Qt instead of GTK for 
graphical interfaces, to better fit in with other Qt-based applications.’.

>> Why? AFAICT, there are no git submodules.
> 
> True
> 
>>   Can you test that icons are displayed properly in a pure environment 
> ("guix shell --pure nm-tray -- …»)?
> 
> They are not. I don’t know how to get them to work.

You are using qt-build-system, so in theory the required wrapping is 
done automatically via wrap-all-qt-programs.

Also, with 'icons', I meant 'icons', not .desktop (I've heard at least a 
single case where someone wrote 'icons' but meant .desktop).

> 
>> could you set WITH_MODEMMANAGER_SUPPORT?
> 
> It looks like it’s not yet implemented in the app. Instead, I have 
> removed modem manager from inputs.

OK.

> 
>> 29 сент. 2022 г., в 19:29, Maxime Devos <maximedevos@telenet.be 
>> <mailto:maximedevos@telenet.be>> написал(а):
>>
>>> 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 <team@pantherx.org 
>> <mailto:team@pantherx.org>>
>>
>> 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 
>> <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 
>> <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 
>> <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.
>> <OpenPGP_0x49E3EE22191725EE.asc>

You quoted a bit too much here.

Greetings,
Maxime.

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 929 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]