[Bug 507479] Review Request: moblin-cursor-theme - Moblin X cursors icon theme

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=507479





--- Comment #4 from Christoph Wickert <fedora at christoph-wickert.de>  2009-08-03 16:57:26 EDT ---
OK - MUST: $ rpmlint
/var/lib/mock/fedora-rawhide-x86_64/result/moblin-cursor-theme-0.3-1.fc12.*
2 packages and 0 specfiles checked; 0 errors, 0 warnings.
OK - MUST: named according to the Package Naming Guidelines
OK - MUST: spec file name matches the base package %{name}
OK - MUST: package meets the Packaging Guidelines
OK - MUST: Fedora approved license and meets the Licensing Guidelines: CC-BY-SA
OK - MUST: license field in spec file matches the actual license
OK - MUST: license file included in %doc
OK - MUST: spec is in American English
OK - MUST: spec is legible
FAIL - MUST: source does not matche upstream source by MD5
  Upstream: 4e88ee79b4aafc08e4dc6defc3dadf7d
  Package: 676eef435c190168bf05b51a24f46772
This is most likely because it's a git snapshot, please use the tarball instead
OK - MUST: successfully compiles and builds into binary rpms on x86_64 (noarch
package)
OK - MUST: all build dependencies are listed in BuildRequires (none!)
N/A - MUST: handles locales properly with %find_lang
OK - MUST: not designed to be relocatable
OK - MUST: owns all directories that it creates
OK - MUST: no duplicate files in the %files listing
OK - MUST: permissions on files are set properly, includes %defattr(...)
OK - MUST: package has a %clean section, which contains rm -rf %{buildroot}.
FAIL - MUST: macro usage not consistent: you are using both %{buildroot}/ and
$RPM_BUILD_ROOT
OK - MUST: package contains permissable content
OK - MUST: no large docs
OK - MUST: files included as %doc do not affect the runtime
OK - MUST: does not contain any .la libtool archives.
OK - MUST: does not own files or directories already owned by other
packages. In fact it does own %{_datadir}/icons/moblin/ which is already owned
by moblin-icon-theme, but since none of the packages requires the other this is
ok.
OK - MUST: at the beginning of %install, the package runs rm -rf %{buildroot}.
OK - MUST: all filenames valid UTF-8


SHOULD Items:
OK - SHOULD: If the source package does not include license text(s) as a
separate file from upstream, the packager SHOULD query upstream to include it.
N/A - SHOULD: The description and summary sections in the package spec file
should contain translations for supported Non-English languages, if available.
OK - SHOULD: builds in mock.
OK - SHOULD: package compiles and builds into binary rpms on all supported
architectures.
OK - SHOULD: functions as described.
N/A - SHOULD: scriptlets are sane (no scriptlets used)
N/A - SHOULD: If the package has file dependencies outside of /etc, /bin,
/sbin, /usr/bin, or /usr/sbin consider requiring the package which provides the
file instead of the file itself.


Other items:
OK - latest stable version


Issues:
- Summary: 'Moblin X cursors icon theme' should be just 'Moblin X cursors
theme', because cursors are no icons. On the other hand %decription could be
more elaborate, e. g.: 'This package contains the Moblin X cursors theme.' or
similar.
- don't use a disttag. This is a noarch theme with no dependencies, so it is
not necessary to update it during an release upgrade.
- add NEWS to %doc (README is currently not worth adding)
- '%{__mkdir_p} -p' is a duplication. Please don't use macros for simple things
like %{__cp} or %{__mkdir_p}. You never know if/how these macros are defined.
- use install rather than cp to make sure permissions are correct and to
preserve timestamps
- don't hardcode /usr/share in %install, use %{_datadir}
- Provide the full URL of Source0 and remove the comment on generation of the
tarball. This will also fix the wrong MD5, but the tarball needs to be build
and we can't use 'make dist' here. Instead, we do a few steps manually:

%build
cd pngs
./make.sh

%install
rm -rf %{buildroot}
mkdir -p %{buildroot}/%{_datadir}/icons/moblin/cursors
for file in xcursors/*; do
  install -pm 0644 $file %{buildroot}/%{_datadir}/icons/moblin/cursors
done

Note that you'll also need 
BuildRequires:  xorg-x11-apps
because it provides xcursorgen

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.