Re: [PATCH 3/3] gnu: Duplicity: Update to 0.7.10

[Leo Famulari <leo@famulari.name>]

On Thu, Sep 22, 2016 at 04:31:37PM +1000, Brendan Tildesley wrote:
> Eric Bavier 於 2016-09-21 05:25 寫道:
> > Let's leave out the nnecessary whitespace changes.
> >
> > Could you send an updated patch?
> 
> Ok, I remade it all to look like I didn't just rewrite everything from
> scratch, and added the incorrect indentation back in. Seems odd to
> prefer keeping the patch small over fixing indentation.

Having lots of code changes that don't change the function of the code
makes it difficult for reviewers and Git log readers to understand what
was changed. If a package definition has really bad indentation or style
problems, then I think it's best to fix them in their own commit where
reviewers can easily determine that nothing functional has changed.

Thank you for revising your patches!

> From 15721c677f20156071fe0efb150b3af63f64648c Mon Sep 17 00:00:00 2001
> From: Brendan Tildesley <brendan.tildesley@openmailbox.org>
> Date: Tue, 20 Sep 2016 20:41:30 +1000
> Subject: [PATCH 1/3] gnu: duplicity: Update to 0.7.10.
> 
> * gnu/packages/backup.scm (duplicity): Update to 0.7.10.
> [source]: Remove duplicity-piped-password.patch.
> [source]: Remove duplicity-test_selection-tmp.patch.
> * gnu/local.mk (dist_patch_DATA): Removed above patch file entries.

In general, could you try following our conventions when writing the
changelogs? [0] Reviewers can rewrite them, of course, but we'd prefer
to not have to. For this patch, it would look something like what's
below.  The important thing is to mention all the changes. I look at
`git log` when I am unsure.

Subject: [PATCH 1/3] gnu: duplicity: Update to 0.7.10.

* gnu/packages/backup.scm (duplicity): Update to 0.7.10.
[source]: Remove obsolete patches.
[inputs]: Remove util-linux. Make python2-mock a native-input.
[arguments]: Also substitute path in 'testing/overrides/bin/lftp' in phase
'check-setup'.
* gnu/packages/patches/duplicity-piped-password.patch, 
gnu/packages/patches/duplicity-test_selection-tmp.patch: Delete files.
* gnu/local.mk (dist_patch_DATA): Remove them.

> From aa2ed1cc2e6863f26e675f77ff210b346b8bfd93 Mon Sep 17 00:00:00 2001
> From: Brendan Tildesley <brendan.tildesley@openmailbox.org>
> Date: Thu, 22 Sep 2016 01:45:19 +1000
> Subject: [PATCH 2/3] gnu: duplicity: Use modify-phases.
> 
> * gnu/packages/backup.scm (duplicity): Use modify-phases instead of
>   alist-cons-before.

The patch LGTM, although I'd silently edit the commit message to this
when pushing:

* gnu/packages/backup.scm (duplicity)[arguments]: Use modify-phases instead of
alist-cons-before.

> From 95a4a505cf48627205cfd734b73c74913d88c36c Mon Sep 17 00:00:00 2001
> From: Brendan Tildesley <brendan.tildesley@openmailbox.org>
> Date: Thu, 22 Sep 2016 01:03:32 +1000
> Subject: [PATCH 3/3] gnu: duplicity: Add various backends
> 
> * gnu/packages/backup.scm (duplicity): Add various backends.

* gnu/packages/backup.scm (duplicity)[inputs]: Add python2-lockfile,
python2-pexpect, python2-paramiko, python2-pycrypto, python2-botocore,
python2-dropbox, lftp, ncftp, par2cmdline.

This patch doesn't work, because we don't have a python2-dropbox
package. Also, I noticed that without this patch, the first patch (gnu:
duplicity: Update to 0.7.10.) failed because it needs pexpect.

So, whatever is needed for the update should go in that first patch, and
the python2-dropbox patch should be provided at the beginning of this
patch series.

Can you send a revised patch series?

[0] See the text about the ChangeLog format:
https://www.gnu.org/software/guix/manual/html_node/Submitting-Patches.html