[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 #5 from Peter Robinson <pbrobinson at gmail.com>  2009-08-03 18:29:50 EDT ---
> FAIL - MUST: source does not matche upstream source by MD5
>   Upstream: 4e88ee79b4aafc08e4dc6defc3dadf7d
>   Package: 676eef435c190168bf05b51a24f46772

No its not. Read the note at the top of the spec file about how to recreate the
tarball.

> FAIL - MUST: macro usage not consistent: you are using both %{buildroot}/ and
> $RPM_BUILD_ROOT

Fixed.

> 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.

Fixed

> - add NEWS to %doc (README is currently not worth adding)

There is no NEWS file :-)

> - '%{__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.

Fixed

> - use install rather than cp to make sure permissions are correct and to
> preserve timestamps

Used "cp -p" to preserve as its a directory of multiple as per Packaging
Guidelines.

> - don't hardcode /usr/share in %install, use %{_datadir}

Fixed

> - 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:

Updated the source url. I'll update the spec file with the procedure mentioned
above. Actually just wish moblin would actually fix their 'make dist'

> %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  

Added.

-- 
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.