Re: Questionable "cosmetic changes" commits
Hi Bengt! > ┌──┐ > │ "So I removed the comments." │ > └──┘ > Raghav, I think you may not grok the social signalling of a statement like > that :) My apologies! I didn't mean that with a negative connotation. > It sounds like you are overlooking the _social_ need for consensus > in modifying a shared environment. > > Taking a picture off the wall of a shared living room is different > from taking the same picture off the wall in your private room. > > A git commit in a jointly developed FLOSS project is modifying a shared > living room. > (But do what you like in your own git repo ;-) > > The social aspect is not about the technical merits of of your changes, > it's about the difference between joint ownership and private ownership, > and the differences in exercising owner rights. I understand. When the patch was in the mail-list for a while, I assumed folks were okay with the changes. I am glad that Mark shared the concerns. This gave me an opportunity to be in consensus with others. :-) I will be changing my work-flow to prevent this issue from happening again. Regards, RG.
Re: Cosmetic changes commits as a potential security risk (was Re: Questionable "cosmetic changes" commits)
Hi Mark! > Thanks for the explanation. > > Please keep in mind that every comment in Guix was deliberately put > there by a Guix developer, which means that at least one developer > thought the comment was worth including. > > I'm concerned that you felt so confident in your assessment that these > comments were superfluous that you felt justified in removing them > without telling anyone, let alone asking your mentors if they agreed. > > My larger concern is that these removals were effectively hidden within > a commit that ostensibly only rearranged and reindented code. My apologies, I should have mentioned in the commit message. Anyway, I will be deferring from removing any existing comments. > It occurs to me that commits that rearrange or reindent code are a > potential security risk, because they obscure other changes made within > the same commit. Even developers who try to keep an eye on changes > being made to Guix tend to simply *assume* that commits like these are > what they claim to be, because it's too tedious to verify them. > > If we allow unannounced changes to be obscured within "cosmetic changes" > commits without reprimand, we invite the future possibility of > deliberate corruption of our code base via such commits, by attackers > who have compromised our developers' machines or signing keys. I see. I haven't thought about this, but will consider it. Thanks! Regards, RG.
Re: Questionable "cosmetic changes" commits
Hi Chris! > 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. I understand. I didn't mean trivial as in importance but as in reason. Like most common reason why one propagates an input. :-) > 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. Agreed. > 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 Yes, will be useful. Regards, RG.
Re: Questionable "cosmetic changes" commits
Hi Christopher and Raghav, On +2020-12-05 21:54:36 +, Christopher Baines wrote: > > Raghav Gururajan 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. > ┌──┐ │ "So I removed the comments." │ └──┘ Raghav, I think you may not grok the social signalling of a statement like that :) It sounds like you are overlooking the _social_ need for consensus in modifying a shared environment. Taking a picture off the wall of a shared living room is different from taking the same picture off the wall in your private room. A git commit in a jointly developed FLOSS project is modifying a shared living room. (But do what you like in your own git repo ;-) The social aspect is not about the technical merits of of your changes, it's about the difference between joint ownership and private ownership, and the differences in exercising owner rights. > 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 -- Regards, Bengt Richter
Cosmetic changes commits as a potential security risk (was Re: Questionable "cosmetic changes" commits)
Hi Raghav, I asked: >> Do you have an explanation for why you are removing comments in your >> "cosmetic changes" commits? "Raghav Gururajan" replied: > 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. Thanks for the explanation. Please keep in mind that every comment in Guix was deliberately put there by a Guix developer, which means that at least one developer thought the comment was worth including. I'm concerned that you felt so confident in your assessment that these comments were superfluous that you felt justified in removing them without telling anyone, let alone asking your mentors if they agreed. My larger concern is that these removals were effectively hidden within a commit that ostensibly only rearranged and reindented code. * * * It occurs to me that commits that rearrange or reindent code are a potential security risk, because they obscure other changes made within the same commit. Even developers who try to keep an eye on changes being made to Guix tend to simply *assume* that commits like these are what they claim to be, because it's too tedious to verify them. If we allow unannounced changes to be obscured within "cosmetic changes" commits without reprimand, we invite the future possibility of deliberate corruption of our code base via such commits, by attackers who have compromised our developers' machines or signing keys. * * * Having said all of this, I should also say that I truly appreciate your contributions, Raghav, and I'm glad that you are here. Regards, Mark
Re: Questionable "cosmetic changes" commits
Raghav Gururajan 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 signature.asc Description: PGP signature
Re: Questionable "cosmetic changes" commits
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. Regards, RG.
Re: Questionable "cosmetic changes" commits
I wrote: > I just hacked up a little script to determine which ordering is more > common. For simplicity, it only considers top-level declarations of the > form (define-public (package ...)). To be more precise, it only considers packages of that form where the '...' contains both 'home-page' and 'description' fields, so it excludes most packages that inherit from another package. Mark
Re: Questionable "cosmetic changes" commits
Hi Raghav, "Raghav Gururajan" writes: > Yeah, my brain laterally connects fields of different package > definitions. Like a spread-sheet, where each columns are different > package definitions and each row is a fields of a package's > definition. > > For example, if column 1 is glib and column 2 is gtk+, my brain spots > pattern or errors when [arguments] field of both the packages are in > the same row. Let's say [arguments] field of glib pack-def (column) is > in 4th place (row); and; if the 4th place (row) of gtk+ pack-def > (column) is [propagated-inputs]; my brain goes haywire. So I first do > the cosmetic change of rearranging the fields of gtk+ pack-def to > match with pack-def of gtk+. This is just one example. If your goal is to make the ordering of package fields more consistent across Guix -- which is something that I could support -- I can report that your commits are making that problem worse, not better. One of the common features of your "cosmetic changes" commits is that they all move the 'home-page' field from its conventional place above the 'synopsis' to below the 'description', if it wasn't there already. I just hacked up a little script to determine which ordering is more common. For simplicity, it only considers top-level declarations of the form (define-public (package ...)). Out of 11446 packages of that form in gnu/packages/*.scm, 88% of them (10078) have the 'home-page' field above the 'description' field. So, if consistency of ordering is your goal, you're going in the wrong direction. * * * 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. Mark
Re: Questionable "cosmetic changes" commits
Hi Raghav, first, let me say that as far as I'm concerned, you did nothing wrong--although it caused a lot of work for you to do the rearranging in the first place (and also some work for us). Guix irregularities also annoy the hell out of me. You can check out some earlier patches by me (when I joined the project) in the archives. I wanted (and still want!) to rearrange a LOT of stuff--because, let's face it, Guix's module layout in general is a mess. But for better or for worse, it's not easy to do anything about it and also keep backward compatibility with what external projects expect of Guix and also keep using the tools that we have been using so far (git, diff, patch etc). So even though I still would prefer Guix module layout not to suck, it cannot be changed in the short term. So I know where you are coming from. But in general, arguing about *formatting* is bike-shedding at its best, especially in a language where formatting does not matter. However, we do use textual diff, blame and merge tools--and those do not understand the tree structure of a Lisp program at all. Rearranging stuff especially will (and did) cause diff and patch to mistake the insertion point for changes. (It will patch it wrong if applied out of order or when skipping patches--and will often NOT fail) So for example it's very difficult to leave off the cosmetic patch and just apply the non-cosmetic patches that came later. Furthermore, understand that the package fields (and guix record fields in general) can refer to previously defined fields--so a package with reordered fields is NOT necessarily semantically equivalent to the original one. Try these in guix repl: (let ((name "OUTER")) (package-version (package (name "INNER") (version name) (source #f) (build-system #f) (synopsis #f) (description #f) (license #f) (home-page #f (that gives "INNER") vs (let ((name "OUTER")) (package-version (package (version name) (name "name") (source #f) (build-system #f) (synopsis #f) (description #f) (license #f) (home-page #f (that gives "OUTER") That means each cosmetic patch of you required some extra manual review effort by me in order to make sure that this does not introduce semantic changes. That said, if people post reformatting patches (especially if part of a patchset that was presumably already tested by that person) I usually do not say anything about it because I don't want to cause extra work that is useful to nobody (and potentially invalidate all the testing done). FWIW, I do find it strange that Lisp projects, despite using a minimal-syntax language (mostly in order to conserve its regular tree structure), do not usually automatically format source code as they check in, but Go projects, using the prime example of an irregular C-like language, DOES usually use code formatters automatically when checking in. That is some strange reversal of strengths that I wouldn't have expected. pgp8cS_qBS01E.pgp Description: OpenPGP digital signature
Re: Questionable "cosmetic changes" commits
Hi Ryan! >> I can tell you that those cosmetic changes I made were 100% irrational, >> useless and noisy. > > That's certainly a way to frame it, but I'd like to hold some space for the > idea that the things we > neuroatypical people do to manage and satisfy our own unusual perspectives > are not irrational or > useless if they make sense to us and serve a purpose for us. +1 I meant to say that, the actions were irrational by itself. But when contrasted with right reasons, we can see why sometimes an irrational act can be a rational act, when it benefits the agent's overall outcome. >> Due to [clinical conditions], if the packages I am editing is not it in a >> specific way, I am unable >> to focus properly. That too, if I am working on multiple package definitions >> and if each pack-defs >> are arranged in different way, it is very very hard for me. > > That is an interesting use-case I hadn't considered, but a valid one, and it > gives me ideas. I'm > going to experiment with some tools to make this feasible. Thanks Ryan! Yeah, my brain laterally connects fields of different package definitions. Like a spread-sheet, where each columns are different package definitions and each row is a fields of a package's definition. For example, if column 1 is glib and column 2 is gtk+, my brain spots pattern or errors when [arguments] field of both the packages are in the same row. Let's say [arguments] field of glib pack-def (column) is in 4th place (row); and; if the 4th place (row) of gtk+ pack-def (column) is [propagated-inputs]; my brain goes haywire. So I first do the cosmetic change of rearranging the fields of gtk+ pack-def to match with pack-def of gtk+. This is just one example. > Consider the tooling for Unison[1] which reduces code churn in a number of > unique ways.[2] It can > store programs as abstract syntax trees rather than plain text files, and > it's content-addressed, > referring to functions by their hashes rather than their fixed names. As a > programmer that gives > you the freedom to choose names and language syntax that suit you without > imposing on others to > follow suit. That's very interesting. I'll have a look. > Due to the functional paradigm and technical structure of Guix, I think we > can build some of the > same capabilities in our tooling. Like Unison functions, our package > definitions are reduced to a > declarative content-addressed form, from which a textual definition could be > generated again using > a reverse process. By such a process, you & I could edit package definitions > in whatever textual > form suits us individually without having to agree on anything, not even the > names of symbols. Then > we can use a tool to automatically canonicalize our code into the form most > widely acceptable to > the community when it's time to submit a patch. +1 > Taking this idea to its logical conclusion & building on Guile's > multi-language-syntax paradigm, I > can picture using a futuristic tool to edit package definitions in Emacs lisp > or JavaScript, again > without requiring that anybody upstream adopt those languages or even > recognize that I'm using > them. > > I'll see what I can hack together for a naïve implementation of this concept > and present it for > review. Thank you Ryan! >> Moving forward, I will try hard not to do this. But can I ask you all to >> allow these cosmetic >> commits for my existing projects (i.e. commits from wip-desktop and pidgin >> patch-set) please? > > It doesn't bug me, but I know it does bug other people and imagine we want to > avoid creating > unnecessary review work for people who follow the commit stream. I see. >> I understand that these commits were a burden to everyone and my sincere >> apologies for that. > > I appreciate the grace and consideration you brought to your response & all > your contributions to > Guix! :-) > [1] https://www.unisonweb.org > [2] > https://www.unisonweb.org/2020/04/10/reducing-churn/#definitions-getting-renamed-or-moved Regards, RG.
Re: Questionable "cosmetic changes" commits
On December 4, 2020, Raghav Gururajan wrote: > I can tell you that those cosmetic changes I made were 100% > irrational, useless and noisy. That's certainly a way to frame it, but I'd like to hold some space for the idea that the things we neuroatypical people do to manage and satisfy our own unusual perspectives are not irrational or useless if they make sense to us and serve a purpose for us. > Due to [clinical conditions], if the packages I am editing is not it > in a specific way, I am unable to focus properly. That too, if I am > working on multiple package definitions and if each pack-defs are > arranged in different way, it is very very hard for me. That is an interesting use-case I hadn't considered, but a valid one, and it gives me ideas. I'm going to experiment with some tools to make this feasible. Consider the tooling for Unison[1] which reduces code churn in a number of unique ways.[2] It can store programs as abstract syntax trees rather than plain text files, and it's content-addressed, referring to functions by their hashes rather than their fixed names. As a programmer that gives you the freedom to choose names and language syntax that suit you without imposing on others to follow suit. Due to the functional paradigm and technical structure of Guix, I think we can build some of the same capabilities in our tooling. Like Unison functions, our package definitions are reduced to a declarative content- addressed form, from which a textual definition could be generated again using a reverse process. By such a process, you & I could edit package definitions in whatever textual form suits us individually without having to agree on anything, not even the names of symbols. Then we can use a tool to automatically canonicalize our code into the form most widely acceptable to the community when it's time to submit a patch. Taking this idea to its logical conclusion & building on Guile's multi- language-syntax paradigm, I can picture using a futuristic tool to edit package definitions in Emacs lisp or JavaScript, again without requiring that anybody upstream adopt those languages or even recognize that I'm using them. I'll see what I can hack together for a naïve implementation of this concept and present it for review. > Moving forward, I will try hard not to do this. But can I ask you all > to allow these cosmetic commits for my existing projects (i.e. commits > from wip-desktop and pidgin patch-set) please? It doesn't bug me, but I know it does bug other people and imagine we want to avoid creating unnecessary review work for people who follow the commit stream. > I understand that these commits were a burden to everyone and my > sincere apologies for that. I appreciate the grace and consideration you brought to your response & all your contributions to Guix! [1] https://www.unisonweb.org [2] https://www.unisonweb.org/2020/04/10/reducing-churn/#definitions- getting-renamed-or-moved
Re: Questionable "cosmetic changes" commits
Hello Mark and Others! Thank you for your concern. I can tell you that those cosmetic changes I made were 100% irrational, useless and noisy. I have clinical OCD [1] and ADHD [2], for which I regularly take Fluoxetine and Methylphenidate to keep things under control. Due to this, if the packages I am editing is not it in a specific way, I am unable to focus properly. That too, if I am working on multiple package definitions and if each pack-defs are arranged in different way, it is very very hard for me. I can tell you that these cosmetic changes are merely the product of my mental-illness and not of my mental-faculties. Moving forward, I will try hard not to do this. But can I ask you all to allow these cosmetic commits for my existing projects (i.e. commits from wip-desktop and pidgin patch-set) please? I understand that these commits were a burden to everyone and my sincere apologies for that. [1] https://www.nimh.nih.gov/health/topics/obsessive-compulsive-disorder-ocd/index.shtml [2] https://www.nimh.nih.gov/health/topics/attention-deficit-hyperactivity-disorder-adhd/index.shtml Regards, RG.
Re: Questionable "cosmetic changes" commits
Hi Ryan, Mark, et al, On +2020-12-02 20:13:56 +, Ryan Prior wrote: > Hi Mark! > > On December 2, 2020, Mark H Weaver wrote: > > We all have our own personal preferences of how best to indent scheme > > code, but if more of us adopted the habit of needlessly reordering > > fields and reindenting code of every package we touch, as one of us > > seems to have done, it could get out of hand quickly. > > I don't particularly hold any opinion about stylistic commits except > that I prefer tools like gofmt, Python's Black and standard.js which > enforce uniform code style, and would use such a tool for my Guile code > if it exists. Agree. As Tobias points out, it exists inside emacs. But I like small independent tools for specific things. Especially if they compose well. > > I do think it's important to acknowledge that the commits written by > Raghav were part of his internship and advised by his mentors who signed > off on the commits, so it's not like these changes were unsolicited and > materialized out of nowhere. I find myself with schizophrenic sympathies here :) On the one hand: 1. Don't make unnecessary work for me. 2. If it ain't broke, don't fix it. 3. Mother appreciates help in the kitchen, but don't touch the fine crystal! (Mother: "That's precious, from your great grandmother, only for special occasions. You know what cut glass crystal like that is worth?" Smarty Kid: "Buying or selling?" :) 4: At work, sign for janitorial services: DO NOT MOVE ANYTHING ON MY DESK!! (arrangement of untidy mess encodes part of my workflow state). On the other hand: 1. I habitually use emacs scheme-mode indenting as well as shell-script-mode, and find it a useful lens to reveal the meaning of the code, and my errors. 2. I prefer to read pretty-printed code, and wouldn't mind if all submitted code automatically were canonicalized in a well-defined way. I use emacs indenting because it is right there when I am editing. 3. NOT to canonicalize can obscure dumb errors, and that can be a smokescreen for worse. 4. For code, it is the abstract syntax tree that matters (in telling the machine what to do), not cosmetics, but cosmetics matter in making info human-sensible. In conclusion: 1. I want the best of all worlds, so I always like to pick a la carte, unless the Chef is three-flowers-plus, or I am budget-constrained. 2. I lean towards wanting a standard pretty-print canonicalization, if it doesn't make work for me ;) 3. diff has options to ignore white-space differences, etc., so I wonder what tools need what options to ease Mark's pain, (until everything is canonicalized, when that pain should disappear anyway). 4. Canonicalization should actually help make reviewing easier, and more effective, IWT. 5. I would like better factoring of functionality, so that e.g. I wouldn't have to start emacs (I know, some never leave :) to canonicalize a file the way scheme-mode does. Unix philosophy? (E.g., don't give cat a new built-in option like -nA, but realize how easily it could compose with a factored-out scheme-source canonicalizer. Some of you could probably write a bash script to use emacs as a filter, but is that a sensible way to go? Probably, if you are a fluent hacker and you need a tool in a hurry, but not for deciding separate and separable distribution components. (I think I got my nostalgic ideas of components from Meccano kit from the '40s ;) tl;DR: 1. I vote for running all code to be submitted through a standard pretty-printing canonicalizer. It could even inject TODO stubs for missing synopses, and doc strings (as an option :) -- Regards, Bengt Richter
Re: Questionable "cosmetic changes" commits
Hi Ryan, Ryan Prior writes: > I do think it's important to acknowledge that the commits written by > Raghav were part of his internship and advised by his mentors who signed > off on the commits, so it's not like these changes were unsolicited and > materialized out of nowhere. If those cosmetic changes were solicited by his mentors, then please consider my comments to be directed at them. Incidentally, my comments were not meant to shame anyone. I very much appreciate Raghav's contributions to Guix. I'm merely pointing out a practice that I think is suboptimal and should be discouraged. Thanks, Mark
Re: Questionable "cosmetic changes" commits
Hi Martin, Am 02.12.20 um 19:55 schrieb Mark H Weaver: > I think that commits like this are best avoided for several reasons. […] > Should I change those things back the next time I update that package? My main project (PyInstaller) has the policy to not accept any white-space changes and reformatting, except this is done due to functional change. I'm very picky about this, since digging through a pile of cosmetic changes to find the cause of a functional change is cumbersome. For guix packages, finding the cause of a functional change is less of an issue. Anyhow, each change contributes to the load of data to be transferred. This not only is an issue for people with low bandwidths, but also contributes to climate change. The later is especially true, since "guix pull" pulls all changes on all machines using guix - so each single change will be downloaded many thousand time for next 10 years. -- Regards Hartmut Goebel | Hartmut Goebel | h.goe...@crazy-compilers.com | | www.crazy-compilers.com | compilers which you thought are impossible |
Re: Questionable "cosmetic changes" commits
Ryan Prior 写道： I don't particularly hold any opinion about stylistic commits except that I prefer tools like gofmt, Python's Black and standard.js which enforce uniform code style, and would use such a tool for my Guile code if it exists. Guix already has a uniform code formatter -- GNU Emacs :-) (After applying .dir-locals.el, and possibly invoked with etc/indent-code.el.) Kind regards, T G-R signature.asc Description: PGP signature
Re: Questionable "cosmetic changes" commits
Hi Mark! On December 2, 2020, Mark H Weaver wrote: > We all have our own personal preferences of how best to indent scheme > code, but if more of us adopted the habit of needlessly reordering > fields and reindenting code of every package we touch, as one of us > seems to have done, it could get out of hand quickly. I don't particularly hold any opinion about stylistic commits except that I prefer tools like gofmt, Python's Black and standard.js which enforce uniform code style, and would use such a tool for my Guile code if it exists. I do think it's important to acknowledge that the commits written by Raghav were part of his internship and advised by his mentors who signed off on the commits, so it's not like these changes were unsolicited and materialized out of nowhere.
Questionable "cosmetic changes" commits
Hello fellow Guix,
In recent months there have been several "cosmetic changes" commits that
I find questionable. These commits reorder package fields and reindent
code that was already ordered and indented according to our conventions,
apparently in order to match the author's personal preferences.
These commits also sometimes remove useful comments, although this is
not mentioned in the commit logs and not easily noticed in the diffs
amongst the noise of reordering and reindentation.
Here are some recent examples:
https://git.sv.gnu.org/cgit/guix.git/commit/?id=c3264f9e100ad6aefe5216002b68f3bfdcf6be95
https://git.sv.gnu.org/cgit/guix.git/commit/?id=416b1b9f56b514677660b56992cea1c78e00f519
https://git.sv.gnu.org/cgit/guix.git/commit/?id=2394b76bd223f5903e325f1f0e6d6b6fe69d00ed
https://git.sv.gnu.org/cgit/guix.git/commit/?id=d829c348f8a3c4de7e0dedeb4f96913357ae5294
https://git.sv.gnu.org/cgit/guix.git/commit/?id=7c63d0e29f3b33719a2e581d07125a9d03a0ec88
https://git.sv.gnu.org/cgit/guix.git/commit/?id=51e7e72bca9622560cde27db785b2d3e3fe058ae
https://git.sv.gnu.org/cgit/guix.git/commit/?id=0bb718c1b2c8df29ec85a81f002c54061c05ef65
https://git.sv.gnu.org/cgit/guix.git/commit/?id=b168f2ba53b938e1b322c79e5bfa47fcc506b803
https://git.sv.gnu.org/cgit/guix.git/commit/?id=d257697dc1ab4d2a278415d75b057c072f4660ec
https://git.sv.gnu.org/cgit/guix.git/commit/?id=b96961c9d25dd4efeaecf33813f9025282b25869
https://git.sv.gnu.org/cgit/guix.git/commit/?id=98f1951bb93524ed7bd212d884ed17ef21d4653d
I've included below one example for illustration. The following commit
removes two comments (one about licensing details, and another
explaining why 'libffi' is needed in propagated-inputs), moves the
'home-page' field from its conventional place above the 'synopsis' to
below the 'description' (a common feature among this recent batch of
commits), swaps the order of the 'inputs' and 'native-inputs' fields,
and reindents several fields to be more vertically oriented, as
illustrated by the following change:
___ (native-search-paths
(list (search-path-specification
___ (variable "GI_TYPELIB_PATH")
___ (files '("lib/girepository-1.0")
was changed to:
___ (native-search-paths
(list
_ (search-path-specification
__ (variable "GI_TYPELIB_PATH")
__ (files '("lib/girepository-1.0")
I think that commits like this are best avoided for several reasons.
Most importantly, they make merges between branches more error prone.
We all have our own personal preferences of how best to indent scheme
code, but if more of us adopted the habit of needlessly reordering
fields and reindenting code of every package we touch, as one of us
seems to have done, it could get out of hand quickly. For example, my
personal preference would be to reverse the indentation change of the
'native-search-paths' field shown above, and to move the 'home-page'
field back above the 'synopsis' to match our usual conventions. Should
I change those things back the next time I update that package?
What do other people think?
Regards,
Mark
>From c3264f9e100ad6aefe5216002b68f3bfdcf6be95 Mon Sep 17 00:00:00 2001
From: Raghav Gururajan
Date: Thu, 24 Sep 2020 08:57:59 -0400
Subject: [PATCH] gnu: gobject-introspection: Make some cosmetic changes.
* gnu/packages/glib.scm (gobject-introspection): Make some cosmetic changes.
Signed-off-by: Danny Milosavljevic
---
gnu/packages/glib.scm | 45 ++-
1 file changed, 23 insertions(+), 22 deletions(-)
diff --git a/gnu/packages/glib.scm b/gnu/packages/glib.scm
index 43523e516d..a4fa6faefb 100644
--- a/gnu/packages/glib.scm
+++ b/gnu/packages/glib.scm
@@ -429,17 +429,20 @@ dynamic loading, and an object system.")
(package
(name "gobject-introspection")
(version "1.62.0")
-(source (origin
- (method url-fetch)
- (uri (string-append "mirror://gnome/sources/"
- "gobject-introspection/" (version-major+minor version)
- "/gobject-introspection-" version ".tar.xz"))
- (sha256
- (base32 "18lhglg9v6y83lhqzyifc1z0wrlawzrhzzxx0a3h1g7xaz97xvmi"))
- (patches (search-patches
- "gobject-introspection-cc.patch"
- "gobject-introspection-girepository.patch"
- "gobject-introspection-absolute-shlib-path.patch"
+(source
+ (origin
+ (method url-fetch)
+ (uri
+(string-append "mirror://gnome/sources/"
+ "gobject-introspection/" (version-major+minor version)
+ "/gobject-introspection-" version ".tar.xz"))
+ (sha256
+(base32 "18lhglg9v6y83lhqzyifc1z0wrlawzrhzzxx0a3h1g7xaz97xvmi"))
+ (patches
+(search-patches
+ "gobject-introspection-cc.patch"
+ "gobject-introspection-girepository.patch"
+ "gobject-introspection-absolute-shlib-path.patch"
