From: Christopher Baines <mail@cbaines.net>
To: Raghav Gururajan <raghavgururajan@disroot.org>
Cc: guix-devel@gnu.org, Ryan Prior <ryanprior@hey.com>
Subject: Re: Questionable "cosmetic changes" commits
Date: Sat, 05 Dec 2020 21:54:36 +0000 [thread overview]
Message-ID: <87lfebrk9v.fsf@cbaines.net> (raw)
In-Reply-To: <b12415505e213533b59f157212c21cd5@disroot.org>
[-- Attachment #1: Type: text/plain, Size: 1862 bytes --]
Raghav Gururajan <raghavgururajan@disroot.org> writes:
> Hi Mark!
>
>> Meanwhile, you've only provided a rationale for 1 out of 3 of the kinds
>> of changes made in these commits.
>>
>> Do you have an explanation for why you are removing comments in your
>> "cosmetic changes" commits? For example, the following two commits
>> remove comments that explain why 'propagated-inputs' are needed:
>>
>> https://git.sv.gnu.org/cgit/guix.git/commit/?id=c3264f9e100ad6aefe5216002b68f3bfdcf6be95
>> https://git.sv.gnu.org/cgit/guix.git/commit/?id=416b1b9f56b514677660b56992cea1c78e00f519
>>
>> What's your rationale for doing this? Am I the only one here who finds
>> this practice objectionable? It's not even mentioned in the commit logs.
>
> I think the comments are useful for non-trivial cases. In these
> definitions, the inputs were propagated because they were mentioned in
> .pc files. Propagation because of pkg-config is trivial. So I removed
> the comments.
In the context of writing Guix packages, propagating the necessary
inputs to support other packages finding the library via pkg-config is a
serious thing, not trivial. If it breaks, dependent packages will likely
change in behaviour or stop building entirely.
As for the comments, personally, I think the reasons behind propagated
inputs are individual enough and important enough to each package that
it's useful to write them down, even if that comment is "these things
are referenced by the .pc file". That way others looking at the package
definition don't have to wonder or try and dig through the Git history
to find information about what's going on.
Anyway, I think the most useful output from this discussion is amending
or adding to the packaging guilelines to cover this:
https://guix.gnu.org/manual/en/html_node/Packaging-Guidelines.html
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]
next prev parent reply other threads:[~2020-12-05 21:54 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-02 18:55 Questionable "cosmetic changes" commits Mark H Weaver
2020-12-02 20:13 ` Ryan Prior
2020-12-02 21:27 ` Tobias Geerinckx-Rice
2020-12-02 22:22 ` Mark H Weaver
2020-12-03 3:16 ` Bengt Richter
2020-12-02 21:33 ` Hartmut Goebel
2020-12-04 2:08 ` Raghav Gururajan
2020-12-04 3:30 ` Ryan Prior
2020-12-04 3:58 ` Raghav Gururajan
2020-12-04 15:12 ` Danny Milosavljevic
2020-12-05 6:47 ` Mark H Weaver
2020-12-05 7:06 ` Mark H Weaver
2020-12-05 20:37 ` Raghav Gururajan
2020-12-05 21:54 ` Christopher Baines [this message]
2020-12-05 23:42 ` Bengt Richter
2020-12-20 7:07 ` Raghav Gururajan via Development of GNU Guix and the GNU System distribution.
2020-12-05 23:29 ` Cosmetic changes commits as a potential security risk (was Re: Questionable "cosmetic changes" commits) Mark H Weaver
2020-12-20 6:55 ` Questionable "cosmetic changes" commits Raghav Gururajan via Development of GNU Guix and the GNU System distribution.
2020-12-20 7:00 ` Cosmetic changes commits as a potential security risk (was Re: Questionable "cosmetic changes" commits) Raghav Gururajan via Development of GNU Guix and the GNU System distribution.
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://guix.gnu.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87lfebrk9v.fsf@cbaines.net \
--to=mail@cbaines.net \
--cc=guix-devel@gnu.org \
--cc=raghavgururajan@disroot.org \
--cc=ryanprior@hey.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/guix.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).