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

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 58125 in the body.
You can then email your comments to 58125 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Pavel Shlyak <p.shlyak <at> pantherx.org>
To: guix-patches <at> gnu.org
Subject: [PATCH] gnu: Add nm-tray 
Date: Tue, 27 Sep 2022 23:49:57 +0300

[0001-gnu-Add-nm-tray.patch (application/octet-stream, attachment)]

Message #8 received at 58125 <at> debbugs.gnu.org (full text, mbox):

From: Maxime Devos <maximedevos <at> telenet.be>
To: Pavel Shlyak <p.shlyak <at> pantherx.org>, 58125 <at> debbugs.gnu.org
Subject: Re: [bug#58125] [PATCH] gnu: Add nm-tray
Date: Thu, 29 Sep 2022 18:29:12 +0200

[Message part 1 (text/plain, inline)]

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

[OpenPGP_0x49E3EE22191725EE.asc (application/pgp-keys, attachment)]

[OpenPGP_signature (application/pgp-signature, attachment)]

Message #11 received at 58125 <at> debbugs.gnu.org (full text, mbox):

From: Pavel Shlyak <p.shlyak <at> pantherx.org>
To: Maxime Devos <maximedevos <at> telenet.be>
Cc: 58125 <at> debbugs.gnu.org
Subject: Re: [bug#58125] [PATCH] gnu: Add nm-tray
Date: Wed, 5 Oct 2022 17:46:50 +0300

[Message part 1 (text/plain, inline)]

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

I didn’t get you

> 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

> 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

> 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

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

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



> 29 сент. 2022 г., в 19:29, Maxime Devos <maximedevos <at> 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 <at> 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> 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.
> <OpenPGP_0x49E3EE22191725EE.asc>

[Message part 2 (text/html, inline)]

[0001-gnu-Add-nm-tray.patch (application/octet-stream, attachment)]

[Message part 4 (text/html, inline)]

Message #14 received at 58125 <at> debbugs.gnu.org (full text, mbox):

From: Maxime Devos <maximedevos <at> telenet.be>
To: Pavel Shlyak <p.shlyak <at> pantherx.org>
Cc: 58125 <at> debbugs.gnu.org
Subject: Re: [bug#58125] [PATCH] gnu: Add nm-tray
Date: Wed, 5 Oct 2022 19:03:54 +0200

[Message part 1 (text/plain, inline)]

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 <at> telenet.be 
>> <mailto:maximedevos <at> 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 <at> pantherx.org 
>> <mailto:team <at> 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.

[OpenPGP_0x49E3EE22191725EE.asc (application/pgp-keys, attachment)]

[OpenPGP_signature (application/pgp-signature, attachment)]

Message #19 received at 58125-done <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Pavel Shlyak <p.shlyak <at> pantherx.org>
Cc: 58125-done <at> debbugs.gnu.org, Maxime Devos <maximedevos <at> telenet.be>
Subject: Re: bug#58125: [PATCH] gnu: Add nm-tray 
Date: Mon, 10 Oct 2022 10:44:36 +0200

Hi,

Pavel Shlyak <p.shlyak <at> pantherx.org> skribis:

> From a9473671cbbcf7c97495a4d65070006feb80f09b Mon Sep 17 00:00:00 2001
> From: Pavel Shlyak <p.shlyak <at> pantherx.org>
> Date: Tue, 27 Sep 2022 23:45:08 +0300
> Subject: [PATCH] gnu: Add nm-tray
>
> * gnu/packages/networking.scm (nm-tray): New variable.

I passed it through ‘guix style’, tweaked the description as Maxime
suggested, and committed.

Thanks!

Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.