Bug 468765 - Review Request: hydrogen-drumkits - Additional DrumKits for Hydrogen
Review Request: hydrogen-drumkits - Additional DrumKits for Hydrogen
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Peter Robinson
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-10-27 17:07 EDT by Orcan Ogetbil
Modified: 2008-12-21 18:40 EST (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-12-21 18:36:54 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
pbrobinson: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Orcan Ogetbil 2008-10-27 17:07:33 EDT
Spec URL: http://oget.fedorapeople.org/review/hydrogen-drumkits.spec
SRPM URL: http://oget.fedorapeople.org/review/hydrogen-drumkits-0-0.20080907.1.fc9.src.rpm
Description: 
A collection of additional drumkits for the
Hydrogen advanced drum machine for GNU/Linux.

Notes:
Hydrogen website provides a large collection of drumkits. However, most of these drumkits are licenseless. In this SRPM, I only included a subset which contains drumkits licensed under either GPL or LinuxTag Green OpenMusic License. The latter is not among the "Good Licenses" list in the guidelines hence I am blocking FE-Legal.
The full text of this license can be found in the SRPM, and also at
http://openmusic.linuxtag.org/green.html
Comment 1 Tom "spot" Callaway 2008-10-31 09:08:47 EDT
The Green license is free, as long as clause VI isn't invoked (which it is not here). The Yellow license isn't (but you're not using it).

I've added the Green license to the table as "Green OpenMusic", so you need to adjust your license tag to:

License: GPLv2+ and Green OpenMusic

Lifting FE-Legal, thanks for your due diligence here.
Comment 2 Orcan Ogetbil 2008-11-02 15:24:22 EST
Thank you spot for taking care of the license issue.

I updated the files:
SPEC: http://oget.fedorapeople.org/review/hydrogen-drumkits.spec
SRPM: http://oget.fedorapeople.org/review/hydrogen-drumkits-0-0.20080907.2.fc9.src.rpm
Comment 3 Orcan Ogetbil 2008-11-06 13:28:10 EST
I realized that the some sources I used fail to match the upstream sha1sums. I thought maybe the upstream updated their files but their page still shows the old timestamps??

Anyways, I created a new SRPM that has the correct sha1sums.

SPEC: http://oget.fedorapeople.org/review/hydrogen-drumkits.spec
SRPM:
http://oget.fedorapeople.org/review/hydrogen-drumkits-0-0.20080907.3.fc9.src.rpm
Comment 4 Till Maas 2008-11-07 05:40:29 EST
THis probably creates a empty README.ColomboAcousticDrumkit file:

iconv -f iso-8859-1 -t utf8 README.ColomboAcousticDrumkit \
                         -o README.ColomboAcousticDrumkit

If you want to use iconv, afaik you have to output it to a temporary file, use touch to restore the timestamp of the temporary file and then rename it.
Comment 5 Orcan Ogetbil 2008-11-07 18:16:27 EST
Thanks for the reminder. I forgot to do that.
The way I did it does not produce an empty file, but it doesn't preserve the time stamp either. Now it is fixed.

The new files are:
http://oget.fedorapeople.org/review/hydrogen-drumkits.spec
http://oget.fedorapeople.org/review/hydrogen-drumkits-0-0.20080907.4.fc9.src.rpm

Do you think I should change the (E)VR to
Version: 20080907 (or 2008.09.07)
Release: 4

since this is a package made for Fedora specifically? I don't think the upstream will ever produce such a compilation package of drumkits.
Comment 6 Peter Robinson 2008-12-16 17:49:39 EST
I think the date is fine (20080907 style) but I think the version shouldn't be 0. Is the format of the drumkits dependant on particular versions hydrogen?
Comment 7 Orcan Ogetbil 2008-12-16 18:45:48 EST
Yes, they only work for hydrogen 0.93 and up. They will release a new beta for hydrogen-0.94 very soon, and I will update that for rawhide. The drumkits will work with hydrogen-0.94.

Anyhow, should I use 20080907 for version?
Comment 8 Peter Robinson 2008-12-16 18:56:43 EST
If they're dependant on a particular version of hydrogen then I would use the version of hydrogen as the version with a build rev as the date like you currently have. That way its easy for the user to tell what version of hydrogen they need (or at least the minimum) and the date the drumkits were packaged.
Comment 9 Orcan Ogetbil 2008-12-16 19:07:30 EST
But these drumkits will work for both hydrogen-0.93 and hydrogen-0.94.

hydrogen-0.93 is in the stable repo and rawhide right now.
hydrogen-0.94 will be in rawhide soon. but hydrogen-0.93 will remain in the stable repo. So should I use 0.93 for the drumkits?
Comment 10 Peter Robinson 2008-12-16 19:24:13 EST
I think 0.93 is fine. it specifies the minimum version. and then you have a requires for >= minimum version.
Comment 11 Orcan Ogetbil 2008-12-16 19:38:31 EST
OK I will use
Version: 0.93
Release: 1.20080907

inspired by this guideline: http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Snapshot_packages

Anything else that needs fixed?
Comment 12 Peter Robinson 2008-12-16 20:02:31 EST
It looks mostly OK. You should move from %{buildroot} to $RPM_BUILD_ROOT as its the standard now. Other than that once there's an updated package I'll do a full review.
Comment 13 Orcan Ogetbil 2008-12-16 22:04:24 EST
I did the changes you asked. I also found another free drumkit in hydrogen's forums. I added it to the package:

SPEC: http://oget.fedorapeople.org/review/hydrogen-drumkits.spec
SRPM: http://oget.fedorapeople.org/review/hydrogen-drumkits-0.9.3-1.20080907.fc10.src.rpm

%changelog: 0.9.3-1.20080907
- Change versioning
- Some cosmetics on the SPEC file
- Added Asma Davul (GPLv3) to the drumkits
Comment 14 Till Maas 2008-12-17 04:25:17 EST
(In reply to comment #12)
> It looks mostly OK. You should move from %{buildroot} to $RPM_BUILD_ROOT as its
> the standard now. Other than that once there's an updated package I'll do a
> full review.

There is nothing wrong with using %{buildroot} in general:
https://fedoraproject.org/wiki/Packaging/Guidelines#macros
Comment 15 Peter Robinson 2008-12-18 07:50:53 EST
APPROVED

+ rpmlint output

This is OK due to legal ACK above (might be nice to file a bug against rpmlint to get it added)

rpmlint -i hydrogen-drumkits-0.9.3-1.20080907.fc10.src.rpm 
hydrogen-drumkits.src: W: invalid-license Green OpenMusic
The value of the License tag was not recognized.  Known values are: "AAL",
"Adobe", "ADSL", "AFL", "AGPLv1", "AGPLv3", "AMPAS BSD", "ARL", "ASL 1.0",
"ASL 1.0+", "ASL 1.1", "ASL 1.1+", "ASL 2.0", "ASL 2.0+", "APSL 2.0", "APSL
2.0+", "Artistic 2.0", "Artistic clarified", "BitTorrent", "Boost", "BSD",
"BSD with advertising", "CATOSL", "CeCILL", "CeCILL-B", "CeCILL-C", "CDDL",
"CNRI", "CPAL", "CPL", "Condor", "Copyright only", "Crystal Stacker", "DOC",
"ECL 1.0", "ECL 2.0", "eCos", "EFL 2.0", "EFL 2.0+", "Entessa", "EPL", "ERPL",
"EU Datagrid", "Fair", "FTL", "Giftware", "GL2PS", "Glide", "gnuplot", "GPL+",
"GPL+ or Artistic", "GPLv1", "GPLv2 or Artistic", "GPLv2+ or Artistic",
"GPLv2", "GPLv2 with exceptions", "GPLv2+", "GPLv2+ with exceptions", "GPLv3",
"GPLv3 with exceptions", "GPLv3+", "GPLv3+ with exceptions", "IBM", "IJG",
"ImageMagick", "iMatix", "Imlib2", "Intel ACPI", "Interbase", "ISC", "Jabber",
"JasPer", "LBNL BSD", "LGPLv2", "LGPLv2 with exceptions", "LGPLv2+", "LGPLv2+
or Artistic", "LGPLv2+ with exceptions", "LGPLv3", "LGPLv3 with exceptions",
"LGPLv3+", "LGPLv3+ with exceptions", "libtiff", "LLGPL", "LPL", "LPPL",
"mecab-ipadic", "MIT", "MIT with advertising", "Motosoto", "MPLv1.0",
"MPLv1.0+", "MPLv1.1", "MPLv1.1+", "NCSA", "NetCDF", "NGPL", "NOSL", "Naumen",
"Netscape", "Nokia", "OpenLDAP", "OpenPBS", "OReilly", "OSL 1.0", "OSL 1.0+",
"OSL 1.1", "OSL 1.1+", "OSL 2.0", "OSL 2.0+", "OSL 2.1", "OSL 2.1+", "OSL
3.0", "OSL 3.0+", "OpenSSL", "OReilly", "Phorum", "PHP", "psutils", "Public
Domain", "Python", "Qhull", "QPL", "RiceBSD", "RPSL", "Ruby", "SCRIP",
"Sendmail", "Sleepycat", "SISSL", "SLIB", "SPL", "TCL", "Teeworlds", "TMate",
"UCD", "VOSTROM", "Vim", "VNLSL", "VSL", "W3C", "WTFPL", "wxWidgets", "Xerox",
"xinetd", "YPLv1.1", "Zend", "ZPLv1.0", "ZPLv1.0+", "ZPLv2.0", "ZPLv2.0+",
"ZPLv2.1", "ZPLv2.1+", "zlib", "zlib with acknowledgement", "CDL", "FBSDDL",
"GFDL", "IEEE", "OFSFDL", "Open Publication", "Public Use", "CC-BY", "CC-BY-
SA", "CC-BY-ND", "DSL", "EFML", "Free Art", "GeoGratis", "OAL", "Arphic",
"Baekmuk", "Bitstream Vera", "Hershey", "Liberation", "Lucida", "mplus",
"OFL", "STIX", "Utopia", "XANO", "Redistributable, no modification permitted",
"Freely redistributable without restriction".

1 packages and 0 specfiles checked; 0 errors, 1 warnings.

+ package name satisfies the packaging naming guidelines
  lots of different drumkits but naming matches dependent package
+ specfile name matches the package base name
+ package should satisfy packaging guidelines
+ license meets guidelines and is acceptable to Fedora
  Green OpenMusic ACKed by legal
+ license matches the actual package license

+ %doc includes license file
+ spec file written in American English
+ spec file is legible
+ upstream sources match sources in the srpm

86dbdb8d2f9b12690e92211d36c6fe7d  Classic-626.h2drumkit
c08d5093fc28a30a7542f0c89493e807  Classic-808.h2drumkit
cb11827e185ab5a6906967901495884b  ColomboAcousticDrumkit.h2drumkit
df1bd778148cc25d8f63a8cc7aa91fcb  ElectricEmpireKit.h2drumkit
f953edf3f4227d786df59b544370e379  HardElectro1.h2drumkit
f445c60d4625a6bfe6bb9dbac1ac0aa7  K-27_Trash_Kit.h2drumkit
4c895d59c3bc5f3322d14789de345be2  Millo-Drums_v.1.h2drumkit
2da5b8ee87bce3e67464c61ba0b722dd  Millo_MultiLayered2.h2drumkit
79ca7360784ec72959aa07c3c584d76c  Millo_MultiLayered3.h2drumkit
a9c305829cd23c28ffd1647cb5c0bdfd  VariBreaks.h2drumkit
88196a71b20a656e97e70071569dd82f  asma_davul.tar.gz

+ package successfully builds on at least one architecture
  tested using koji scratch build
+ BuildRequires list all build dependencies
n/a %find_lang instead of %{_datadir}/locale/*
n/a binary RPM with shared library files must call ldconfig in %post and %postun
+ does not use Prefix: /usr
+ package owns all directories it creates
n/a no duplicate files in %files
+ %defattr line
+ %clean contains rm -rf $RPM_BUILD_ROOT
+ consistent use of macros
+ package must contain code or permissible content
n/a large documentation files should go in -doc subpackage
+ files marked %doc should not affect package
n/a header files should be in -devel
n/a static libraries should be in -static
n/a packages containing pkgconfig (.pc) files need 'Requires: pkgconfig'
n/a libfoo.so must go in -devel
n/a devel must require the fully versioned base
n/a packages should not contain libtool .la files
n/a packages containing GUI apps must include %{name}.desktop file
+ packages must not own files or directories owned by other packages
+ %install must start with rm -rf %{buildroot} etc.
+ filenames must be valid UTF-8

Optional:

+ if there is no license file, packager should query upstream
n/a translations of description and summary for non-English languages, if
available
+ reviewer should build the package in mock/koji
n/a the package should build into binary RPMs on all supported architectures
n/a review should test the package functions as described
+ scriptlets should be sane
n/a pkgconfig files should go in -devel
+ shouldn't have file dependencies outside /etc /bin /sbin /usr/bin or
/usr/sbin
Comment 16 Orcan Ogetbil 2008-12-18 11:20:46 EST
Thanks!

New Package CVS Request
=======================
Package Name: hydrogen-drumkits
Short Description: Additional DrumKits for Hydrogen
Owners: oget
Branches: F-9 F-10 devel
InitialCC: lkundrak green
Comment 17 Kevin Fenzi 2008-12-20 23:27:24 EST
cvs done.
Comment 18 Fedora Update System 2008-12-21 01:34:17 EST
hydrogen-drumkits-0.9.3-1.20080907.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/hydrogen-drumkits-0.9.3-1.20080907.fc10
Comment 19 Fedora Update System 2008-12-21 01:35:41 EST
hydrogen-drumkits-0.9.3-1.20080907.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/hydrogen-drumkits-0.9.3-1.20080907.fc9
Comment 20 Fedora Update System 2008-12-21 18:36:50 EST
hydrogen-drumkits-0.9.3-1.20080907.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 21 Fedora Update System 2008-12-21 18:40:07 EST
hydrogen-drumkits-0.9.3-1.20080907.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

Note You need to log in before you can comment on or make changes to this bug.