From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp2.migadu.com ([2001:41d0:403:4876::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms1.migadu.com with LMTPS id oGM3M6s0RWaRXgAAe85BDQ:P1 (envelope-from ) for ; Thu, 16 May 2024 00:18:20 +0200 Received: from aspmx1.migadu.com ([2001:41d0:403:4876::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp2.migadu.com with LMTPS id oGM3M6s0RWaRXgAAe85BDQ (envelope-from ) for ; Thu, 16 May 2024 00:18:19 +0200 X-Envelope-To: larch@yhetil.org Authentication-Results: aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=incana.org header.s=key1 header.b=p99e0DJ3; spf=pass (aspmx1.migadu.com: domain of "guix-patches-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="guix-patches-bounces+larch=yhetil.org@gnu.org"; dmarc=pass (policy=none) header.from=gnu.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1715811499; h=from:from:sender:sender:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:resent-cc:resent-from:resent-sender: resent-message-id:in-reply-to:in-reply-to:references:references: list-id:list-help:list-unsubscribe:list-subscribe:list-post: dkim-signature; bh=2xyBmLigYL9+90rWS4yBR/iEERQPw6qZKKcGRWH1Xg4=; b=R1mq03bXtP4mZpOp8dhsFe59ky/gNav41B7Jz+Gbp5zj3eXouV32LWIW8UDb4DR0E4OoXS 0/ZJQQvVUI0Q0KOo2ybfEVKJIAaL9isMsoCpvEFNzB+vLtCvHe164URBf21oF2bYX1i8wS Ft5GHqZKWJV0xNf2d+shqGzdv+i9RSH9W2+CTC7tV9MJ+i9o8bFWpEOxqeQpynJUoSNOoi F4vdUzN7H4wuMlhIqvg+Ep74vi0HGw04v0Ek/7AyIplhCBTBj35snYP/G4C/B9baoaYWfM pmK/BtkBrLCUhCdSTjrWsvqvxbrn0vxnMm3tgqhSIzBfPPVeVy3kTtBoZycRXw== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=incana.org header.s=key1 header.b=p99e0DJ3; spf=pass (aspmx1.migadu.com: domain of "guix-patches-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="guix-patches-bounces+larch=yhetil.org@gnu.org"; dmarc=pass (policy=none) header.from=gnu.org ARC-Seal: i=1; s=key1; d=yhetil.org; t=1715811499; a=rsa-sha256; cv=none; b=WfMdLsZqsq58oMgx121O75ZlZnw7b0HuY0GVouDObbu7vtcFnC4dPsaDLrxijZWjArXycs gLp6VIfGZQBLnkMS4OmS7C53mGZe4x4/dDYgYN/90Z/2s+UsUjdv2aDmVFV/oPUehOcwzg tztVGEmIzGmotgGmU/HNZJxyJaisFDinc5Cr8GCVenExIIWAMnfQ/DnfbLuPptYQ2PCR4f ZHCnvU/xtGhQ65MW/Awq70wF6nHvCKtabgL95LzY4smjJYRO4/ikgQbdmvasPdfC6ZIC3R fzUC5eAHQxfjAl5AJhQ9pc6JIXpeLNU63u6ryOCtBRHCjM+YBZOZHj41mCBfiA== Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id 695CED679 for ; Thu, 16 May 2024 00:18:19 +0200 (CEST) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1s7Mwl-0001XH-1u; Wed, 15 May 2024 18:18:03 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1s7Mwi-0001X4-Ui for guix-patches@gnu.org; Wed, 15 May 2024 18:18:01 -0400 Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1s7Mwi-0004AO-Mc for guix-patches@gnu.org; Wed, 15 May 2024 18:18:00 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1s7Mwk-00008P-7O for guix-patches@gnu.org; Wed, 15 May 2024 18:18:02 -0400 X-Loop: help-debbugs@gnu.org Subject: [bug#70845] [PATCH v2] services: Add fancontrol-service-type. References: <20240509154032.5047-1-neox@gnu.org> In-Reply-To: <20240509154032.5047-1-neox@gnu.org> Resent-From: Juliana Sims Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Wed, 15 May 2024 22:18:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 70845 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: neox@gnu.org Received: via spool by 70845-submit@debbugs.gnu.org id=B70845.1715811440508 (code B ref 70845); Wed, 15 May 2024 22:18:02 +0000 Received: (at 70845) by debbugs.gnu.org; 15 May 2024 22:17:20 +0000 Received: from localhost ([127.0.0.1]:44769 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1s7Mw4-000088-Gt for submit@debbugs.gnu.org; Wed, 15 May 2024 18:17:20 -0400 Received: from out-180.mta1.migadu.com ([95.215.58.180]:30352) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1s7Mw1-000080-Te for 70845@debbugs.gnu.org; Wed, 15 May 2024 18:17:18 -0400 Date: Wed, 15 May 2024 18:16:25 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=incana.org; s=key1; t=1715811400; h=from:from:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type; bh=2xyBmLigYL9+90rWS4yBR/iEERQPw6qZKKcGRWH1Xg4=; b=p99e0DJ3PZxJ0PoZyI8OqAnTFNbbsAB9EW1JvNXWNrb3fpwzBb3h8fHXNZupq5Fkr9H1KP Sd4oQYeIhTtx4JTQ0XQ28LxWF3nJgVqr+L7f4yehhsHEdJrM91VydzRogVWpXc54PjMI8I jq0PsA8IwfmYWJY4F/7G2Nr8Pdh95RDE3TsH5pTTNX5qq1b8DTs16lHZMfmBgEISEEd7o4 8/J80b6D+Nkedp2E4nPWRbUqE0EI3y892Ltwn5c8vI98oO28qgEemyyDo5UOwbFTBCk3ZM tCZTog2YUrfn8hrtB0tGXKOwJF8292HcGtATCIdx6LJQzngQTpyy5OdZBPLzfw== X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. Message-Id: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: guix-patches@gnu.org List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Juliana Sims , 70845@debbugs.gnu.org Reply-to: b8ea66dd0b5edd9e8d345a0beda3198049ab6231.1715609187.git.neox@gnu.org X-ACL-Warn: , Juliana Sims via Guix-patches From: Juliana Sims via Guix-patches via Errors-To: guix-patches-bounces+larch=yhetil.org@gnu.org Sender: guix-patches-bounces+larch=yhetil.org@gnu.org X-Migadu-Country: US X-Migadu-Flow: FLOW_IN X-Spam-Score: -6.87 X-Migadu-Queue-Id: 695CED679 X-Migadu-Scanner: mx10.migadu.com X-Migadu-Spam-Score: -6.87 X-TUID: hXLdxiGYUpHa Hi Adrien, Thanks for this patch! It looks pretty good, though I do have a few small bits of feedback. 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. 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. That's the only real critique I have here. Consider the rest of this email fun ideas rather than review per se :) We had an out-of-band exchange about this patch that I'll summarize here for the record. I echoed the sentiments of the reviewer who suggested exposing the fancontrol package so that users could change it. Your response was that the configuration is generated by pwmconfig and therefore it wouldn't make sense to provide a Scheme interface to configuration. 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. All that aside, you should be able to expose the package setting to users without using define-configuration. Best, Juli