Spec URL: http://people.redhat.com/clalance/rubygem-compass-960-plugin/rubygem-compass-960-plugin.spec SRPM URL: http://people.redhat.com/clalance/rubygem-compass-960-plugin/rubygem-compass-960-plugin-0.10.4-1.fc14.src.rpm Description: The 960 Grid System is an effort to streamline web development workflow by providing commonly used dimensions, based on a width of 960 pixels. http://960.gs/
Hi Chris, by coincidence, I did the same package today before I noticed you did that yesterday :) So you can take a look for comparison [1]. A few things I noticed in you spec file: * Wrong license * site_libdir is useless for gem packages * you do not provide -doc subpackage, although I am not sure if we should not disable the documentation for this gem at all * %defattr is no more needed except EPEL-4 * BuildRoot is deprecated, except EPEL-5 and older I guess * Clean section is not required anymore * There is better to use %{geminstdir} macro in %files section [1] http://people.redhat.com/vondruch/rubygem-compass-960-plugin.spec
And you did not specified ruby(abi)
(In reply to comment #1) > Hi Chris, > > by coincidence, I did the same package today before I noticed you did that > yesterday :) So you can take a look for comparison [1]. A few things I noticed > in you spec file: > > * Wrong license Oops, fixed. > * site_libdir is useless for gem packages Yep, removed. > * you do not provide -doc subpackage, although I am not sure if we should not > disable the documentation for this gem at all Hm, I'm really fine either way. I guess it does make sense to have the -doc package, like other rubygems. I've taken that bit from yours and put it into mine. > * %defattr is no more needed except EPEL-4 > * BuildRoot is deprecated, except EPEL-5 and older I guess > * Clean section is not required anymore Heh, I had no idea. But re-reading the packaging guidelines, I see that this is indeed the case. I've removed all 3 of these. > * There is better to use %{geminstdir} macro in %files section Fixed. (In reply to comment #2) > And you did not specified ruby(abi) Right, fixed. I've re-uploaded the SPEC and SRPM to the same place; can you take a look when you get a chance? Thanks, Chris Lalancette
(In reply to comment #3) > > * you do not provide -doc subpackage, although I am not sure if we should not > > disable the documentation for this gem at all > > Hm, I'm really fine either way. I guess it does make sense to have the -doc > package, like other rubygems. I've taken that bit from yours and put it into > mine. I just checked the content of the documentation when I did the package myself and there was virtually nothing, that why I doubt if it would not be better to disable it at all, but on the other hand, it is not installed by default. So lets keep it the way it is now. And I found yet another minor nit. You keep the .gemspec file which is in gem folder but it has no meaning in our package, so I suggest to remove it: %exclude %{geminstdir}/%{gemname}.gemspec And the final one, you are using "Requires: rubygems" where it should be better to use the virtual provide "Requires: ruby(rubygems)". The same apply for BR. Otherwise the package looks good. So I APPROVE it.
Thanks Vit. I'll fix those two nits before I import the package. Chris Lalancette
New Package SCM Request ======================= Package Name: rubygem-compass-960-plugin Short Description: Compass compatible Sass port of 960.gs Owners: clalance vondruch Branches: InitialCC:
Git done (by process-git-requests). Note to Vit: Take ownership of Review bugs you're offically working on and/or approving.
(In reply to comment #7) > Git done (by process-git-requests). > > Note to Vit: Take ownership of Review bugs you're offically working on > and/or approving. Hmm, strange ... assigned doesn't mean assigned to me? :) Will try to do better next time.