hydrogen packaging

[Jaromír Mikeš]

2016-11-24 23:38 GMT+01:00 James Cowgill <jcowgill at debian.org>:
> 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>:

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