[Merge] ~kick-d/ubuntu/+source/amavisd-new:master into ~ubuntu-server-dev/ubuntu/+source/amavisd-new:master

Mon Jan 18 18:09:07 UTC 2016

Review: Needs Fixing

Hi Pierre,

The merge itself looks good, thanks. Just a few things to fix regarding documentation. See inline comments about an inaccuracy in your change to README.Debian, extra changelog whitespace, a listed remaining change actually being a new change, and a suggestion to make our decision about the previous conffile error a little clearer.

For your next package merge (don't worry about it this time for amavisd-new), please could you also push a logical/<version> tag that corresponds to the old/ubuntu version? This would make it easier to review and is the process I'm pushing for the server team.

Please add commits to your branch to fix the three or four things above, and I'll look again. Thanks!

Diff comments:

> diff --git a/debian/README.Debian b/debian/README.Debian
> index 7a5e2e3..3b1b729 100644
> --- a/debian/README.Debian
> +++ b/debian/README.Debian
> @@ -24,6 +24,9 @@ Read-write conffiles: /etc/amavis/conf.d/
>    15-av_scanners:		AV scanner interface configuration
>    15-content_filter_mode:	Use this to re-enable spamassassin/av checks
>    20-debian_defaults:		Commonly modified settings
> +  21-ubuntu_defaults:		Additional Ubuntu specific changes
> +  22-amavisd-new-postfix	Amavisd-new-postfix configuration for anti-spam/virus

As we decided to continue not to ship 22-amavisd-new-postfix, and this is what your merge is doing, we shouldn't be telling the user via README.Debian it exists, because the user (installing from fresh) will not be able to find it. I'd be happy if either you don't mention it at all (like before), or explain the full story.

> +  40-policy_banks:		DKIM whitelist
>    50-user:			Place your overrides here, if you want
>  
>  If the package detects legacy config files, it renames them adding a
> diff --git a/debian/changelog b/debian/changelog
> index 4efc46e..b8ea48d 100644
> --- a/debian/changelog
> +++ b/debian/changelog
> @@ -1,3 +1,23 @@
> +amavisd-new (1:2.10.1-1ubuntu1~ppa9) xenial; urgency=medium
> +
> +  * Merge from Debian stable, (LP: #1227195) remaining changes:

That's fine. We can just merge the two bugs together.

> +    + Add information in README.Debian about Ubuntu specific changes
> +    + Ubuntu configuration changes in 21-ubuntu_defaults
> +      - Reduce email responses for virus/blocked mail so as not to be a
> +        backscatter source by default
> +      - Enable DKIM checking by default
> +    + Include policy-bank of known good domains for DKIM whitelisting
> +      in 40-policy_banks
> +    + debian/control: drop altermime and ripole to Suggests after discussions
> +      with the server team. (LP: #992879)
> +    + amavisd-new-postfix configuration for anti-spam/virus
> +    + debian/rules: Add some comments about 22-amavisd-new-postfix file being forgotten 

This is not a remaining change, so it is not accurate to put it under a section labeled "remaining changes". Please make it a top-level bullet point.

> +      since 2.6.5-0ubuntu1. Decision has been made to keep this state, by commenting out,
> +      to avoid a different behavior which could disturb various configuration tools
> +      assumptions.

I suggest that you add that the reason for the decision is that we expect to eliminate the delta in Xenial+1.

> +
> + -- Pierre-André MOREY <pierre-andre.morey at canonical.com>  Tue, 12 Jan 2016 15:40:02 +0100
> +
>  amavisd-new (1:2.10.1-1) unstable; urgency=medium
>  
>    * [486e81e] Update watchfile
> @@ -50,12 +70,57 @@ amavisd-new (1:2.9.0-1) unstable; urgency=low
>  
>   -- Alexander Wirt <formorer at debian.org>  Sat, 10 May 2014 22:51:45 +0200
>  
> -amavisd-new (1:2.7.1-2) unstable; urgency=low
> +amavisd-new (1:2.7.1-2ubuntu3) quantal; urgency=low
> +
> +  * debian/control: drop altermime and ripole to Suggests after discussions
> +    with the server team. (LP: 992879)
> +
> + -- Jamie Strandboge <jamie at ubuntu.com>  Fri, 21 Sep 2012 10:23:40 -0500
> +
> +amavisd-new (1:2.7.1-2ubuntu2) quantal; urgency=low
> +
> +  * Fix invalid 'spf-policyd_time_limit' parametere in
> +    amavisd-new-postifx (LP: #996569)
>  
> + -- Ante Karamatic <ivoks at ubuntu.com>  Sun, 29 Jul 2012 10:31:31 +0200
> +
> +amavisd-new (1:2.7.1-2ubuntu1) quantal; urgency=low
> +
> +  * Merge from Debian unstable, remaining changes:
> +    - amavisd-new-postfix configuration for anti-spam/virus
> +    - Add information in README.Debian about Ubuntu specific changes
> +    - Ubuntu configuration changes in 21-ubuntu_defaults
> +      - Reduce email responses for virus/blocked mail so as not to be a
> +        backscatter source by default
> +      - Enable DKIM checking by default
> +    - Include policy-bank of known good domains for DKIM whitelisting
> +      in 40-policy_banks
> +    - Drop use of hardening-wrapper, since we're no longer compiling
> +      anything.
> +
> + -- Logan Rosen <logatronico at gmail.com>  Tue, 10 Jul 2012 22:05:35 -0400
> +
> +amavisd-new (1:2.7.1-2) unstable; urgency=low
> + 

You seem to have inserted extra whitespace here that was never present in Debian nor in the previous Ubuntu upload. This is making my diff unnecessarily noisy. Please remove it so this part of the changelog is exactly the merged changelog with no other changes, apart from your new entry at the top.

>    * [02ccf50] Add free lha decompresser (Closes: #677692)
>  
>   -- Alexander Wirt <formorer at debian.org>  Sat, 16 Jun 2012 19:04:24 +0200
>  
> +amavisd-new (1:2.7.1-1ubuntu1) quantal; urgency=low
> +
> +  * Merge from Debian unstable, remaining changes:
> +    - amavisd-new-postfix configuration for anti-spam/virus
> +    - Add information in README.Debian about Ubuntu specific changes
> +    - Ubuntu configuration changes in 21-ubuntu_defaults
> +      - Reduce email responses for virus/blocked mail so as not to be a
> +        backscatter source by default
> +      - Enable DKIM checking by default
> +    - Include policy-bank of known good domains for DKIM whitelisting
> +      in 40-policy_banks
> +  * Drop use of hardening-wrapper, since we're no longer compiling anything.
> +
> + -- Steve Langasek <steve.langasek at ubuntu.com>  Tue, 01 May 2012 05:40:33 +0000
> +
>  amavisd-new (1:2.7.1-1) unstable; urgency=low
>  
>    * [a1fea9f] Imported Upstream version 2.7.1

-- 
https://code.launchpad.net/~kick-d/ubuntu/+source/amavisd-new/+git/amavisd-new/+merge/282326
Your team Ubuntu Server Developers is subscribed to branch ~ubuntu-server-dev/ubuntu/+source/amavisd-new:master.