On Sun, Feb 2, 2014 at 1:22 AM, Markus Koschany <a...@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 -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org