Bug 453412 (gtk-aurora-engine)
Summary: | Review Request: gtk-aurora-engine - Aurora theme engine for gtk2 | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Tuomas Mursu <tuomas.mursu> |
Component: | Package Review | Assignee: | John Anderson <john.e.anderson> |
Status: | CLOSED DUPLICATE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | low | ||
Version: | rawhide | CC: | besser82, christoph.wickert, fedora-package-review, felix, itamar, john.e.anderson, metherid, notting, pahan |
Target Milestone: | --- | Flags: | john.e.anderson:
fedora-review?
|
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2010-03-23 08:34:06 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
Tuomas Mursu
2008-06-30 13:53:16 UTC
I'm looking for sponsorship as well, so I can't do a full review, but I'd like to point out a few things. 1) It would be nice if you could link directly to the source. I notice http://www.gnome-look.org/CONTENT/content-files/56438-Aurora-1.4.tar.bz2 seems to work for the main source file. 2) Add a -q to your %setup 3) The permissions in source1 are kind of screwy. You can fix them in the files section with something like this: %attr(755, -, root) %{_datadir}/themes/* %attr(644, -, root) /usr/share/themes/Aurora-Midnight/gtk-2.0/gtkrc %attr(644, -, root) /usr/share/themes/Aurora/gtk-2.0/gtkrc %attr(644, -, root) /usr/share/themes/Aurora-looks/gtk-2.0/gtkrc Otherwise, it looks good to me. It builds and installs fine against rawhide. Are you interested in making these fixes, otherwise I would be interested in working on this package? Yeah I'm interested, just been kinda busy lately :) I'll upload fixed packages in a few days. Well well, it didn't take that long. I changed %prep too so that everything stays under one directory. Spec URL: http://koti.kapsi.fi/~darkon/rpm/rev/gtk-aurora-engine/1/gtk-aurora-engine.spec SRPM URL: http://koti.kapsi.fi/~darkon/rpm/rev/gtk-aurora-engine/1/gtk-aurora-engine-1.4-1.fc9.src.rpm Looking pretty good, a few things: 1) You can safely remove the "Requires: gtk2", gtk2-devel should take care of that 2) I looked at the names of other gtk engine packages, and they all followed the format gtk-engine-name I think gtk-engine-aurora would be a better fit 3) I see a minor ownership problem for %{_libdir}/gtk-2.0, see below 4) I would consider putting the themes in a subpackage Please remember to increment the version and update the changelog NEEDSWORK for now. MUST Items: OK - MUST: rpmlint must be run on every package. The output should be posted in the review. 3 packages and 0 specfiles checked; 0 errors, 0 warnings. FIX - MUST: The package must be named according to the Package Naming Guidelines The rest of the gtk engines in Fedora follow the format gtk-engine-name, I would stick with that and change to gtk-engine-aurora OK - MUST: The spec file named in the format %{name}.spec OK - MUST: The package must meet the Packaging Guidelines OK - MUST: The package must be licensed with a Fedora approved license and meet the Licensing Guidelines . OK - MUST: The License field in the package spec file must match the actual license. Ok - MUST: License text included in doc OK - MUST: The spec file must be written in American English. OK - MUST: The spec file for the package MUST be legible. OK - MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. Reviewers should use md5sum for this task. 95e8dab631202504d27eb9925f13317f OK - MUST: The package must successfully compile and build into binary rpms on at least one supported architecture. OK - MUST: Builds on all archs OK - MUST: All build dependencies listed OK - MUST: No locales OK - MUST: No ldconfig needed OK - MUST: Not relocated OK - MUST: A package must own all directories that it creates. OK - MUST: A package must not contain any duplicate files in the %files listing. OK - MUST: Permissions on files must be set properly. OK - MUST: Each package must have a %clean section, which contains rm -rf %{buildroot} ( or $RPM_BUILD_ROOT ). OK - MUST: Each package must consistently use macros, as described in the macros section of Packaging Guidelines . OK - MUST: The package must contain code, or permissable content. OK - MUST: No large docs OK - MUST: If a package includes something as %doc, it must not affect the runtime of the application. To summarize: If it is in %doc, the program must run properly if it is not present. OK - MUST: No headers OK - MUST: No static libraries OK - MUST: No pkgconfig OK - MUST: No library files with a suffix OK - MUST: N/A no devel for devel name OK - MUST: Packages must NOT contain any .la libtool archives, these should be removed in the spec. OK - MUST: Not a GUI app, no .desktop needed FIX - MUST: Packages must not own files or directories already owned by other packages. package owns /usr/lib/gtk-2.0/2.10.0 and /usr/lib/gtk-2.0/2.10.0/engines In files I'd make this change %{_libdir}/gtk-2.0/*/engines/* OK - MUST: At the beginning of %install, each package MUST run rm -rf %{buildroot} ( or $RPM_BUILD_ROOT ). See Prepping BuildRoot For %install for details. OK - MUST: All filenames in rpm packages must be valid UTF-8. SHOULD Items: OK - SHOULD: The reviewer should test that the package builds in mock. OK - SHOULD: The package should compile and build into binary rpms on all supported architectures. OK - SHOULD: The reviewer should test that the package functions as described. Theme works well Is anybody still working on this? *** This bug has been marked as a duplicate of bug 575466 *** |