Hi Jaromír, 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>: >> Ok almost happy with hydrogen package. >> I would like to close also this bug: >> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=591301 >> by adding libportmidi-dev [linux-any], to B-D ... How to do it right >> way in CDBS? >> There are B-D in d/rules file and d/control I guess I should add it to >> one file and regenerate the other. > > I think hydrogen package is ready for upload. > As package has been split I think there is a need upload from DD. > Can someone review and upload please?
Here's a review:
-- d/changelog
> + * Exclude .gitignore file from upstream tarball.
Can this be removed now?
> + * Split package.
I think you should say how the package is split up.
-- d/copyright_hints
The copyright hints are out of date. You should carefully look at
copyright_newhints (after built) and adjust d/copyright accordingly.
-- d/control
> - librubberband-dev,
> - libpulse-dev,
Was this change intentional?
> Package: hydrogen
[...]
> hydrogen-data
You almost certainly want something stricter than this like:
hydrogen-data (= ${source:Version})
> 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.
-- 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?
> 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).
-- d/links
Rename to hydrogen.links?
-- 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:"?
-- d/rules
> +DEB_CLEAN_EXCLUDE=debian/tmp
> +DEB_DESTDIR = $(CURDIR)/debian/tmp/
What is the purpose of this?
> + 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?
> -
> +
> 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).
> # 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.
Thanks,
James
signature.asc
Description: OpenPGP digital signature
_______________________________________________ pkg-multimedia-maintainers mailing list pkg-multimedia-maintainers@lists.alioth.debian.org http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-multimedia-maintainers
