2016-11-24 23:38 GMT+01:00 James Cowgill <jcowg...@debian.org>:
> On 13/11/16 20:23, Jaromír Mikeš wrote:
>> 2016-11-13 19:20 GMT+01:00 Jaromír Mikeš <mira.mi...@gmail.com>:
Hi James,
thank you for your time to review this ... more issues than I thought ;)
> Here's a review:
>
> -- d/changelog
>> + * Exclude .gitignore file from upstream tarball.
> Can this be removed now?
You mean from changelog or removing .gitignore file?
>> + * Split package.
> I think you should say how the package is split up.
Done.
> -- d/copyright_hints
> The copyright hints are out of date. You should carefully look at
> copyright_newhints (after built) and adjust d/copyright accordingly.
I can't find copyright_newhints file ... maybe because I build with pbuilder?
Need advise here ...
> -- d/control
>> - librubberband-dev,
>> - libpulse-dev,
>
> Was this change intentional?
This was done by my wrong manipulation with CDBS
libpulse-dev is back
librubberband-dev not as it is marked as experimental and mature ...
instead rubberband-cli is a dependency ... providing same
functionality
>> Package: hydrogen
> [...]
>> hydrogen-data
>
> You almost certainly want something stricter than this like:
> hydrogen-data (= ${source:Version})
Done
>> Package: hydrogen-data
> [...]
>> Breaks: hydrogen (<= 0.9.6.1-1)
>> Replaces: hydrogen (<= 0.9.6.1-1)
>
> The convention is to use something like (<< 0.9.7-1~) instead (ie << the
> 'fixed' version). Your version would break installations if someone
> modified hydrogen and gave it a local package number. It will also force
> Ubuntu to patch hydrogen since they have 0.9.6.1-1build1.
>
>> Package: hydrogen-doc
> [...]
>> Breaks: hydrogen (<= 0.9.6.1-1)
>> Replaces: hydrogen (<= 0.9.6.1-1)
>
> Same as above.
Fixed for both packages
> -- d/hydrogen.install
>> usr/share/hydrogen/data/img
> It seems that the only reason for putting this in hydrogen instead of
> hydrogen-data is the SVG icon?
Yes
>> data/img/gray/h2-icon.xpm usr/share/pixmaps
>
> Since there is no longer a menu file, installing an XPM icon seems a bit
> pointless (they're ignored by .desktop files).
Fixed ...
> -- d/links
> Rename to hydrogen.links?
Done ...
> -- d/patches/1010-spelling.patch
>> +- ERRORLOG( "Could't locate two Jack input port" );
>> ++ ERRORLOG( "Couldn't locate two Jack input port" );
>
> Should be "...input ports" (plural ports)
>
>> ++[...]more information about you can find here:[...]
>
> Maybe "you can find more information here:"?
Spelling patch improved ... but as you see it is already forwarded
> -- d/rules
>> +DEB_CLEAN_EXCLUDE=debian/tmp
>> +DEB_DESTDIR = $(CURDIR)/debian/tmp/
>
> What is the purpose of this?
This was committed by mistake ... :(
Cleaned.
>> + cp data/doc/manual_en.html data/doc/manual_en.html.bak
>> + touch data/doc/manual.docbook data/doc/tutorial.docbook
>> $(MAKE) -C data/doc
>> touch $@
>> + mv data/doc/manual_en.html.bak data/doc/manual_en.html
>
> Doesn't restoring manual_en.html defeat the purpose of rebuilding the
> documentation?
Exactly ...
>> -
>> +
>> imgstub = data/img/gray/h2-icon
>
> Stray whitespace in diff. All the XPM stuff seems useless now. Possibly
> could drop some build-dependencies after this (I haven't checked).
Removed from rules file
>> # TODO: build-depend on help2man when solved using it with normal builds
>> #CDBS_BUILD_DEPENDS += , librsvg2-bin, netpbm, help2man
>
> This comment and the help2man bit is now obsolete now that upstream have
> a manually written manpage.
comments cleaned ... related B-D removed
best regards
mira
_______________________________________________
pkg-multimedia-maintainers mailing list
pkg-multimedia-maintainers@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-multimedia-maintainers
