[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#844185: RFS: fvwm/2.6.7-1 [ITA] -- f? virtual window manager



On Wed, Nov 16, 2016 at 6:57 PM, Axel Beckert <abe@debian.org> wrote:
> Hi Jaimos,
>

Hi Axel, thanks for responding and looking at my package.

> I've had a short review of your changes and only found a few minor
> nitpicks so far:
>

They were informative and helping me learn, I have updated the package
following their advice.

>>     - New features and bug fixes. For a full list between 2.6.5 and 2.6.7
>>       see /usr/share/doc/fvwm/NEWS.gz
>
> Since the upstream NEWS file is defacto a changelog, it wouldn't be
> bad if the upstream NEWS file would be installed as
> /usr/share/doc/fvwm/changelog.gz

The NEWS file is now used as the changelog.

>>   * Upstream moved from cvs to git. Updated watch file.
>
> Please use https://github.com/fvwmorg/fvwm/releases instead of
> https://github.com/fvwmorg/fvwm/tags in debian/watch. (May need some
> additional tweaking.) Otherwise you would download a Git snapshot of
> the tagged commit instead of the real released tar ball.

Updated. I somehow got fixated on tags due to the old releases being
there, thanks for pointing out releases was far easier to use.

> All the Lintian hardening warnings could be fixed by just adding this
> line to debian/rules (e.g. after the DH_VERBOSE line):
>
> export DEB_BUILD_MAINT_OPTIONS=hardening=+all

Updated

> From the debdiff between the current and your source package:
>
>  %:
> -       dh $@ --with autoreconf
> +       dh $@
>
> Why have you removed "--with autoreconf" while keeping the
> Build-Depends on dh-autoreconf? This change is also not mentioned in
> the debian/changelog entry.
>

Thanks for noticing this,

I had a build problem on the first trial release due to script relying
on .git causing the autoreconf build to fail and removed that to build
from provided files. Upstream fixed it, but after focusing on other
aspects of the package I had forgotten I had done that.

> My suggestion to solve this is to leave debian/rules as it is now,
> remove the build-dependency on dh-autoreconf and use debhelper compat
> level 10 instead (which implies --with autoreconf and --parallel) --
> and mention that in the debian/changelog.
>
> As far as I can see, you can also safely drop the build-dependency on
> autotools-dev as its not used directly and debhelper depends on it
> (and on dh-autoreconf) nowadays. At least it still builds fine in
> pbuilder (without these two build-dependencies, and with debhelper
> compat level 10).
>

Updated. Changed compat to level 10 and the debhelper depends as well,
then dropped the depends and was able to get it to build in pbuilder
as well.

> Despite those are all rather minor issues, I'd be happy if you could
> come up with an updated source package addressing most of these issues
> (at least the debian/watch and missing debian/changelog items).

Done. I have done all the above and also figured out how to get more
output from lintian. fixed some additional spelling errors that were
found as well.

You can find the updated files in the same location
http://fvwmforums.org/fvwm-2.6.7/

Thanks again,

jaimos


Reply to: