Bug 222648
Summary: | Review Request: audacious-plugin-fc - Future Composer plugin for Audacious | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Michael Schwendt <bugs.michael> |
Component: | Package Review | Assignee: | Jason Tibbitts <j> |
Status: | CLOSED ERRATA | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | opensource |
Target Milestone: | --- | Flags: | j:
fedora-review+
kevin: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | 0.2-1 | Doc Type: | Bug Fix |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2007-07-05 19:24:36 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: |
Description
Michael Schwendt
2007-01-15 14:46:22 UTC
====REVIEW CHECKLIST==== * rpmlint not silent: E: audacious-plugin-fc no-changelogname-tag E: audacious-plugin-fc unknown-key GPG#b8af1c54 - package naming guidelines met - spec file name matches %%{name} * packaging guidelines not met: - missing %%changelog entries in spec file - (slightly different build root from the preferred one) - Package licensed with GPL compatible license - %%doc section OK - spec written in American English - spec file legible (only you should remove in the %%install section "mkdir -p $RPM_BUILD_ROOT" - it's not necessary; and as mentioned above, %%changelog missing) - source match upstream (613c456b525d0f5ebce912a981e57264 audacious-plugin-fc-0.1.tar.bz2) - builds successfully on at least i386 FC6 - all build dependences listed - locales handled properly (none present) - no shared libraries - package is not relocatable - package owns all its directories - dirs not owned is owned by required package(s) - no duplicates - file permissions set properly - contains proper %%clean section - macros used consistently - package contains code - no large docs - no -devel subpackage - no pkgconfig files - no libraries with suffix, no static libraries - not a GUI app - package does not own dirs or files owned by other packages - builds in mock ====Things need to be done==== Add %%changelog section to your specfile: http://fedoraproject.org/wiki/Packaging/Guidelines#head-b7d622f4bb245300199c6a33128acce5fb453213 You should change build root to the prefered one: http://fedoraproject.org/wiki/Packaging/Guidelines#head-f196e7b2477c2f5dd97ef64e8eacddfb517f1aa1 As mentioned above, you should remove from the %%install section "mkdir -p $RPM_BUILD_ROOT". It is not necessary. I am not sponsored. This pre-review is to help me get sponsored. Change Source: http://download.sourceforge.net/xmms-fc/audacious-plugin-fc-0.1.tar.bz2 to Source: http://download.sourceforge.net/xmms-fc/audacious-plugin-fc-%{version}.tar.bz2 So you do not need to change the Source with every new version. Consider changing the release-tag to: Release: 1%{?dist} See: http://fedoraproject.org/wiki/Packaging/DistTag And I noticed, that the other audacious plugins in extras have audacious-plugins (plural) as prefix not only audacious-plugin (singular), e.g. audacious-plugins-arts. All valid points. * changed "Buildroot" * changed "Source" * added %changelog for this revision * changed %install Spec URL: http://home.arcor.de/ms2002sep/tmp/audacious-plugin-fc.spec SRPM URL: http://home.arcor.de/ms2002sep/tmp/audacious-plugin-fc-0.1-2.src.rpm * "audacious-plugins" is a collection of multiple plug-ins, where the sub-package names are inherited from the main package. Instead, the packager could have named the sub-packages "audacious-plugin-foo" explicitly. This package contains a single plugin, and its upstream name is audacious-plugin-fc, too. * %{?dist} can be added in CVS. An update only when Audacious 1.3 in FE devel cvs is releaased: Spec URL: http://home.arcor.de/ms2002sep/tmp/audacious-plugin-fc.spec SRPM URL: http://home.arcor.de/ms2002sep/tmp/audacious-plugin-fc-0.2-1.src.rpm Well, 1.3 has been in rawhide for ages now, so.... You shouldn't need a build dependency on pkgconfig; audacious-devel should have its own dependency (and indeed it does). But of course it doesn't hurt anything. It seems there's directory ownership problem; /usr/lib/audacious/Input is owned by audacious-plugins, but they're not required by this package. This seems like the kind of thing the base audacious package should own. Review: * source files match upstream: deafc2c95dc7a1a67a83b53b7871686fa280c8ed18547df51988b648dbe5519b audacious-plugin-fc-0.2.tar.bz2 * package meets naming and versioning guidelines. * specfile is properly named, is cleanly written and uses macros consistently. * summary is OK. * description is OK. * build root is OK. * license field matches the actual license. * license is open source-compatible. * license text included in package. * latest version is being packaged. * BuildRequires are proper. * compiler flags are appropriate. * %clean is present. * package builds in mock (development, x86_64). * package installs properly * debuginfo package looks complete. * rpmlint is silent. * final provides and requires are sane: libfc.so()(64bit) audacious-plugin-fc = 0.2-1 = audacious >= 1.3 libatk-1.0.so.0()(64bit) libaudacious.so.5()(64bit) libcairo.so.2()(64bit) libgcc_s.so.1()(64bit) libgdk-x11-2.0.so.0()(64bit) libgdk_pixbuf-2.0.so.0()(64bit) libglib-2.0.so.0()(64bit) libgmodule-2.0.so.0()(64bit) libgobject-2.0.so.0()(64bit) libgtk-x11-2.0.so.0()(64bit) libmcs.so.1()(64bit) libpango-1.0.so.0()(64bit) libpangocairo-1.0.so.0()(64bit) libpng12.so.0()(64bit) libstdc++.so.6()(64bit) libstdc++.so.6(CXXABI_1.3)(64bit) libstdc++.so.6(GLIBCXX_3.4)(64bit) * %check is not present; no upstream test suite. I've no ability to actually test this as I have no speakers on this machine. * no shared libraries are added to the regular linker search paths. X Ownership issues with /usr/lib/audacious/Input. * doesn't own any directories it shouldn't. * no duplicates in %files. * file permissions are appropriate. * no scriptlets present. * code, not content. * documentation is small, so no -docs subpackage is necessary. * %docs are not necessary for the proper functioning of the package. * no headers. * no pkgconfig files. * no static libraries. * no libtool .la files. > X Ownership issues with /usr/lib/audacious/Input Cannot confirm. The base audacious package requires audacious-plugins. This plugin package requires the base audacious package. *However*, in general, any other multimedia software could implement and use Audacious' plugins, so directory ownership is not trivial to decide on. Probably it could only be fixed with an audacious-base package or similar "bloat". > BR pkgconfig This package directly buildrequires pkgconfig for its m4 code. Eliminating BR based on what some arbitrary package in the dep-chain pulls in cannot be recommended, because e.g. audacious-devel may stop using pkgconfig any time. > The base audacious package requires audacious-plugins. Of course; I didn't chase the dependencies that far. > This package directly buildrequires pkgconfig for its m4 code. OK, that's not the usual usage, and I didn't read the actual code. APPROVED New Package CVS Request ======================= Package Name: audacious-plugin-fc Short Description: Future Composer input plugin for Audacious Owners: bugs.michael Branches: F-7 InitialCC: cvs done. audacious-plugin-fc-0.2-1 has been pushed to the Fedora 7 testing repository. If problems still persist, please make note of it in this bug report. audacious-plugin-fc-0.2-1 has been pushed to the Fedora 7 stable repository. If problems still persist, please make note of it in this bug report. |