From debbugs-submit-bounces@debbugs.gnu.org Sat Sep 24 09:03:40 2022 Received: (at 57598) by debbugs.gnu.org; 24 Sep 2022 13:03:40 +0000 Received: from localhost ([127.0.0.1]:42643 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1oc4ol-0001KM-Nm for submit@debbugs.gnu.org; Sat, 24 Sep 2022 09:03:40 -0400 Received: from eggs.gnu.org ([209.51.188.92]:55108) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1oc4oj-0001K8-7a for 57598@debbugs.gnu.org; Sat, 24 Sep 2022 09:03:38 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]:48228) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oc4od-00061E-2N; Sat, 24 Sep 2022 09:03:31 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=MIME-Version:In-Reply-To:Date:References:Subject:To: From; bh=AxObal4qm450UT8ANxsL5PWP3Hia809U6SipdDNiwh4=; b=BSso5tYjZxv1BuEETzpl z9h01AsF1wkUr3HJZ/FoQXp2izxJIL4sWia3PnKrFPZBwz9/HTR9TclMhpYOX83yh9lQOSeRTDrVG vjEelJ3376mWRCyXtsZyiXZP+tXhO9I4P6isMaNAUsmzrMqb93USuYes7tGAITHriLq1RVziEuMwO jfB+TxqJkrmu+vmEf/dfTU+hcpORGfiSHTMnjLgQ86uaL4sqpF76ubzzw8GOKFNwd0JL41F/XrQKD 3+NVSG/aRMk0mk07H1xYLizkijkSTa7qqO3mpvOA6AKAFBO/kbuTcSLdp4WAfjJklsyQhqEDKAuQZ I2ZQ0IcRvgFvlQ==; Received: from 91-160-117-201.subs.proxad.net ([91.160.117.201]:55887 helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oc4oc-0003q9-E3; Sat, 24 Sep 2022 09:03:30 -0400 From: =?utf-8?Q?Ludovic_Court=C3=A8s?= To: Maxime Devos Subject: Re: bug#57598: [PATCH] doc: Update contribution guidelines on patches, etc. References: <20220905160048.18173-1-maximedevos@telenet.be> <20220909123051.15747-1-maximedevos@telenet.be> Date: Sat, 24 Sep 2022 15:03:27 +0200 In-Reply-To: <20220909123051.15747-1-maximedevos@telenet.be> (Maxime Devos's message of "Fri, 9 Sep 2022 14:30:51 +0200") Message-ID: <87zgeoapr4.fsf_-_@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 57598 Cc: 57598@debbugs.gnu.org, guix-maintainers@gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) Hi! Thanks for this welcome addition! Modulo the cosmetic suggestions below, I think it=E2=80=99s fine. Maintainers, if you have something to say on the guidelines, now=E2=80=99s = the time! https://issues.guix.gnu.org/57598 Maxime Devos skribis: > * doc/contributing.texi (Modifying Sources): Replaced with ... > ("Modifying Sources"): ... this. List more use cases and some principles. > > This patch incorporates some text written by Liliana Marie Prikler. The > structure is based on a proposal by Julien Lepiller. The text has been > revised based on reviews by David Larsson and blake. > > (! remove following text before committing !) You can write comments below the --- and diffstats. That way, =E2=80=98git= am=E2=80=99 will ignore them when applying the patch. > +Guix has tree main ways of modifying the source code of a package, that > +you as a packager may use. These are patches, snippets and phases. ^ =E2=80=9Cmay use: patches, snippets, and build phases.=E2=80=9D > +Each one has its strengths and drawbacks. To decide between the three, s/decide between the three/choose among these/ > +there are a few guiding principles to satisfy simultanuously where > +possible: > + > +@itemize > +@item > +In principle, Guix only has free software; when the upstream source s/In principle, Guix only has free software/Every package in Guix is free software/g > +community (i.e., patterns that appear throughout @code{gnu/packages/...}) Normally such parenthetical expressions go between em dashes: community---i.e., patterns that appear throughout @file{gnu/packages}---and =E2=80=A6 > +To make things more concrete and to resolve conflicts between the > +principles, a few cases have been worked out: > + > +@subsubsection Removing non-free software > +Non-free software has to be removed in snippets; the reason is that > +patches or phases will not work. You can=E2=80=99t have a colon between the section heading. Instead, you c= an write =E2=80=9Ca few cases have been worked out and will be illustrated in = the following sections.=E2=80=9D Section titles should be capitalized. Leave a blank line after the section title. (Same comment for the sections that come after.) Instead of =E2=80=9Cwill not work=E2=80=9D, which is vague, I=E2=80=99d sug= gest more explicit wording: =E2=80=9Cwould not be appropriate because they would expose the offending code.=E2=80=9D > +For patches, the problem is that a patch removing a non-free file > +automatically contains the non-free file@footnote{It has been noted that > +git patches support removing files without including the file in the > +patch in > +@url{https://yhetil.org/guix/8b13e899-eb60-490b-a268-639249698c81@@www.f= astmail.com/}. If > +it is verified that the @command{patch} utility supports such patches, > +this method can be used and this policy adjusted appropriately.}, and we > +do not want anything non-free in Guix even if only in its patches. I=E2=80=99d drop the footnote, it=E2=80=99s already dense enough. > +@code{delete-file-recursively}. There are a few benefits for snippets he= re: > + > +When using snippets, the bundled library does not occur in the source s/snippets here:/snippets here./ s/When using snippets/First, when using snippets/ > +As such, snippets are recommended here. s/are recommended here/are the recommended way to delete non-free material/ > +@subsubsection Fixing technical issues (compilation errors, test failure= s, other bugs ...) In addition to capitalizing, please remove the parenthetical bit. > +@subsubsection Adding new functionality > +To add new functionality, a patch is almost always the most convenient > +choice of the three -- patches are usually multi-line changes, which are s/three -- patches/three---patches/ That=E2=80=99s all I have to say! I think what the text says is fine. It=E2=80=99s dense and rather long tho= ugh, which means that people may overlook things. OTOH it=E2=80=99s structured = so it=E2=80=99s easier to skim through it, and I wouldn=E2=80=99t know what to= remove. Could you send an updated version? Thanks, Ludo=E2=80=99.