Re: Questionable "cosmetic changes" commits

[Danny Milosavljevic]

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.

pgp_IQyDQZAKB.pgp
Description: OpenPGP digital signature