Bug 1002328 (nasacircle)

Avail on fedorapeople now:

SPEC: http://sea.fedorapeople.org/review/grub2-themes-circled/grub2-theme-circled.spec
SRPM: http://sea.fedorapeople.org/review/grub2-themes-circled/grub2-theme-circled-0.2-1.fc17.src.rpm

Review-Preview: http://sea.fedorapeople.org/review/grub2-themes-circled/review-grub2-theme-circled-0.2-1.fc17.src.rpm.html

Hey Simon!

Please file review-request for new packages always against "Package Review" component.  Please be careful about that in future!  :)

I'll take this one.

Cheers,
  Björn

Your spec file contains this:

%install
tar -axf %{sources}

2 issues with this:
- Sources are supposed to be unpackaged in %setup.

- Instead of explicitly invoking tar, sources should be unpackaged with %setup.
  Please google for "rpm %setup", rpm has options to control directory creation
  sequencing and untarring into subdirs.

(OT)I think bugzilla has problem now. Readd CC

@ Björn: Will do

Update:
SPEC: http://sea.fedorapeople.org/review/grub2-themes-circled/grub2-theme-circled.spec
SRPM: http://sea.fedorapeople.org/review/grub2-themes-circled/grub2-theme-circled-0.2-2.fc17.src.rpm

your spec-file has a severe error:

  %changelog
  * Wed Aug 29 2013 - Simon A. Erat (sea) @ sea - 0.2-2
  - Fixed %prep / %install in spec

the last line from %changelog contains unescaped macros, which will make the build-process fail!  It MUST be like this:

  - Fixed %%prep / %%install in spec

Note the doubled percent-symbol (%%) which is used to escape macros inside spec-file properly.  :)

Please fix this.

First, regarding upstream:

It is normal good style that a X.tar.gz extracts in a folder named X.

It is confusing that the upstream project is called one thing and the downloadable file something else ... and the name of the actual themes something third. 

It seems like you are distributing font files without license.

It is claimed that the license is GPL, but that is not clear from the distributed source.

It seems like most of the files are the same for all the themes. Consider storing them in a "common" folder.

(It would be nice if upstream showed some screenshots so we had an idea what to expect.)


It seems like the name of this review request is wrong - it doesn't match the package name.

The prjapppath define do not help readability and it is far from obvious what it is short for. I would just use a wildcard in %files and live with having to specify /boot/grub2 a couple of times.

Do you really think the spec is finished? No summary, no description, a "Place compile commands here" placeholder. Have you tried running rpmlint on the spec?

Remove comment lines that doesn't apply.

There is no reason to clean buildroot in %install.

theme.txt is not a %doc file - but license files are.

"icons" looks like something that should be packaged in a separate subpackage ... or perhaps as a completely separate package that could be reused elsewhere. 

Fonts should reuse existing files in Fedora ... and pf2 files should be built from "source".

ping here.

* Seperated icons as new package (https://bugzilla.redhat.com/show_bug.cgi?id=1004544)
* Fixed spec
* Fixed tarball

Font were created using grub2-mkfont, and i've chosen fonts from the default repos to use.
Solving one issue at a time, i hope the OS icons match the licenses now.

Screenshots: .. err... have to do them again, will place them on:
http://sea.fedorapeople.org/screenshots/grub2-circled-nasa-themes

SPEC: http://sea.fedorapeople.org/Review/grub2-circled-nasa-themes/grub2-circled-nasa-themes.spec
SRPM: http://sea.fedorapeople.org/Review/grub2-circled-nasa-themes/grub2-circled-nasa-themes-0.0.2-3.src.rpm

AH, are you going to support EL5?

And any news here, Björn?

This is an automatic check from review-stats script.

This review request ticket hasn't been updated for some time, but it seems
that the review is still being working out by you. If this is right, please
respond to this comment clearing the NEEDINFO flag and try to reach out the
submitter to proceed with the review.

If you're not interested in reviewing this ticket anymore, please clear the
fedora-review flag and reset the assignee, so that a new reviewer can take
this ticket.

Without any reply, this request will shortly be resetted.

This is an automatic action taken by review-stats script.

The ticket reviewer failed to clear the NEEDINFO flag in a month.
As per https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews
we reset the status and the assignee of this ticket.

This review request is really old. Do you still intend to complete it? If so, please confirm. If not, please close this issue and make it block FE-DEADREVIEW, or do nothing, in which case automation will close the request in one month.

This is an automatic action taken by review-stats script.

The ticket submitter failed to clear the NEEDINFO flag in a month.
As per https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews
we consider this ticket as DEADREVIEW and proceed to close it.