hydrogen packaging

[James Cowgill]

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.mikes at 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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://lists.alioth.debian.org/pipermail/pkg-multimedia-maintainers/attachments/20161124/0b15f3b3/attachment.sig>