Bug#730868: freeciv: New upstream version 2.4.1 available

[Vincent Cheng]

On Sun, Feb 2, 2014 at 1:22 AM, Markus Koschany <apo at gambaru.de> wrote:
> Hi Vincent,
>
> thanks for taking your time to review the package.
>
> On 02.02.2014 09:20, Vincent Cheng wrote:
> [...]
>> Some minor nitpicks (none of which block upload):
>> - since you're building all the client and data binary packages from
>> the same source package, for the client packages, why not just depend
>> on freeciv-data (= ${source:Version}) instead of your current approach
>> ( freeciv-data (<= ${source:Version}), freeciv-data (>=
>> ${source:Upstream-Version}))? The former approach will work equally
>> well for source uploads and won't break on binNMUs, so I'm unsure what
>> the benefit of using the latter is?
>
> I thought that someone who had already downloaded freeciv-data version
> 2.4.1-1 could avoid further downloads and thus save bandwidth. Since the
> arch:all package doesn't change from 2.4.1-1 to 2.4.1-2, it doesn't
> matter which version of the same source package is installed. Using (=
> ${source:Version}) is more strict and forces a download every time.

Practically speaking, if saving bandwidth is a concern, the end user
should be using something like debdelta; apt-get/aptitude will update
all of freeciv's packages at once anyways. I suppose it doesn't really
matter either way though.

>> - you don't need autotools-dev if you're already using dh-autoreconf
>> (you're invoking both helpers in d/rules)
>
> That's right. Here I simply left the line in question intact because I
> wasn't sure whether the package is affected by
>
> http://bugs.debian.org/698765
>
> and whether dh-autoreconf is really a superset of autotools_dev in this
> case.

I'm not an autohell^Wautotools expert, so I can't comment on this, but
I'll note that the same bug report mentions dh-autoreconf(7) saying
that combining the two can have unexpected results as well.

>> - add-keywords-to-desktop-files.patch doesn't have a proper DEP-3
>> header (assuming you've forwarded this upstream, it's missing a link
>> to upstream's bug tracker)
>
> True. I forwarded this patch upstream yesterday.
>
> https://gna.org/bugs/index.php?21573

Great, please add it into the patch header before the next upload.

Cheers,
Vincent