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

Previous Next

Full log

Message #221 received at 38408 <at> debbugs.gnu.org (full text, mbox):

From: Martin Becze <mjbecze <at> riseup.net>
To: Leo Famulari <leo <at> famulari.name>
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: Re: [bug#38408] [PATCH v9 3/8] Added Guile-Semver as a dependency to
 guix
Date: Mon, 23 Mar 2020 05:50:53 -0400

Thanks for the feedback Leo! I will work on this today.

On 3/22/20 4:10 PM, Leo Famulari wrote:
> 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.