GNU bug report logs - #38408
[PATCH 0/3] (WIP) Semantic version aware recusive importer for crates

Previous Next

Full log

View this message in rfc822 format

From: Leo Famulari <leo <at> famulari.name>
To: Martin Becze <mjbecze <at> riseup.net>
Cc: 38408 <at> debbugs.gnu.org, Ludovic Courtès <ludo <at> gnu.org>, Efraim Flashner <efraim <at> flashner.co.il>, jsoo1 <at> asu.edu
Subject: [bug#38408] [PATCH v9 3/8] Added Guile-Semver as a dependency to guix
Date: Sun, 22 Mar 2020 16:10:18 -0400

On Sat, Mar 21, 2020 at 02:35:47PM -0400, Martin Becze wrote:
> A few things got stale and need to be merged so I have regenerated the patch
> set! Let me know if there is anymore things to do.

Alright, I started taking a close look at the patches and they need some
more work. At least, the commit messages need to be completed. The
importer does work, which is amazing, so we are almost there :)

In general, the commit messages need to be rewritten to match our style.
This means they should use complete English sentences with standard
capitalization and punctuation. I'm happy to help if necessary. English
is my native language.

We also should try to match previous commit messages that touch the same
code, because they are good examples. Take this example, the first patch:

> Subject: [PATCH v11 1/9] guix: import: (recursive-import) Allow for version
>  numbers

This commit title should be written like this:

import: utils: 'recursive-import' accepts an optional version parameter.

I learned that by doing `git log guix/import/utils.scm` and reading the
previous commits.

> * guix/import/utils.scm (package->definition): added optional `append-version?`
> * guix/import/utils.scm (recursive-import): added key `version` and
>   moved `repo` to be a key

When changing multiple variables in the same file, the filename can be
mentioned only once. I would write that like this:

* guix/import/utils.scm (recursive-import): Add the VERSION key. Make REPO a key.
(package->definition): Accept an optional 'append-version?' key.

I began to rewrite the rest of the commit message like this:

* guix/import/cran.scm (cran->guix-package): Change the REPO parameter to a key.
(cran-recursive-import): Likewise.
* guix/import/elpa.scm (elpa->guix-package): Likewise.
(elpa-recursive-import): Likewise.
* guix/import/gem.scm (gem-recursive-import): Likewise.
* guix/scripts/import/cran.scm (guix-import-cran): Likewise.
* guix/scripts/import/elpa.scm (guix-import-elpa): Likewise.
* guix/import/opam.scm (opam-recursive-import): Likewise.
* guix/import/pypi.scm (pypi-recursive-import): Likewise.
* guix/import/stackage.scm (stackage-recursive-import): Likewise.


However, I found some issues.

> * guix/import/gem.scm (gem->guix-package): change `repo` to a key

This change does not exist in this commit.

>  tests/elpa.scm               |  3 +-
>  tests/import-utils.scm       |  8 +++--

And these changes are not mentioned in the commit message.

These issues make it hard to review the patches effectively.

What do you think? Can you make these changes?

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.