Subject: Re: [bug#37738] [PATCH] gnu: add lxqt-connman-applet

From: "Reza Alizadeh Majd" <[email-masked]>
Date: Fri, 18 Oct 2019 17:04:24 +0330

Hi Ludo, 

> Nice.  A few comments/questions:
> 
>> +(define-public lxqt-connman-applet
>> +  (let ((commit "3db374eebd8d851f68a50fc5d1ef5fa9478c275e")
>> +        (revision "0"))
> 
> Why did you choose this specific commit?
> 
> Normally we only package upstream releases (tags); we resort to commits
> in between releases only when there’s a good reason to do that, and in
> that cases it’s better to add a comment explaining why we did that.

since the package developers didn’t release a specific version so I 
decided to choose their latest commit in `master` branch. besides that 
I had tested the applet on LXQt desktop and it works without issues. 

>> +      (arguments
>> +        `(#:tests? #f
> 
> Could you add a comment explaining why tests are disabled?

OK, sure, I will submit updated patch ASAP. 

>> +      (home-page "https://github.com/lxqt/lxqt-connman-applet")
>> +      (synopsis "System-tray applet for connman")
>> +      (description "LXQt based system-tray applet for connman")
> 
> Please write a full description:
> 
>  https://guix.gnu.org/manual/en/html_node/Synopses-and-Descriptions.html

unfortunately the original developers didn’t provide any further 
details about this package yet. so is it OK to write a description 
about this package by myself? 

Regards,
Reza