Le mercredi 15 mai 2024 à 18:16 -0400, Juliana Sims a écrit :
> Hi Adrien,

Hi Juliana and thanks a lot for your review. I'm now working on a v3 ;)

> First and foremost, this service needs documentation. Could you add 
> that as well? Speaking of documentation, the 'documentation' field of 
> your Shepherd service has an extraneous bit of whitespace.

Ok, seems sensible to do, I'm working on it and fixing the whitespace issue.

> 
> Is it absolutely vital to use root for this service? Could you instead 
> create a user and usergroup with only the privileges required to run 
> fancontrol? You may need to do something with udev so that works. I'm 
> not sure exactly what privileges are required, but avoiding root seems 
> like a good idea.

I don't actually know if this is vital, but it was the only way to make it work
properly. I did not think about udev though, so I'll try to test things. I agree
that avoiding root is a good idea.

> I don't know this package or how it works, but would it be possible for 
> this service to generate that config automatically when it's first 
> started? If the config is customizable about generation, you could then 
> expose various settings through the Guix service interface for users to 
> modify and rewrite the file for them. That would make using 
> define-configuration worthwhile for more than simply the ability to 
> change the package.

Okay, perhaps more details are needed then. The fancontrol software is made to
control the speed of the fans (CPU heatsinks for example) based on a
configuration that is proper to the fans and that depends on many physical
parameters that might include, sometimes, the environment where the computer is
used. This configuration has to be generated with the pmwconfig software, which
will ask questions to the users (for example "Should we check I2C bus?" or "Did
you hear a fan stopping?"). Since the configuration process is both critical and
interactive, it appears difficult to me to do that automatically. One more thing
is that users should recreate a configuration when the use environment has
changed.

> 
> All that aside, you should be able to expose the package setting to 
> users without using define-configuration.

Ok, thanks for the information. I'll work on that.


Have a nice day everyone and happy hacking!
-- 
Adrien Bourmault
Maintainer, GNU Boot project
Associate member, Free Software Foundation
GPG : 393D4CC68136F39799DA75F295F65F55F682A17A