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=504469 --- Comment #2 from Lubomir Rintel <lkundrak@xxxxx> 2009-06-07 11:00:25 EDT --- Thanks for picking this up (In reply to comment #1) > Some basic comments (please also consider to apply the following > comments to the rest of your rubygem related review requests). > > - Use %global instead of %define: > https://fedoraproject.org/wiki/Packaging/Ruby#Pure_Ruby_packages > https://fedoraproject.org/wiki/PackagingDrafts/global_preferred_over_define > > - Requires: ruby(abi) = 1.8 is missing > https://fedoraproject.org/wiki/Packaging/Ruby#Ruby_Packaging_Guidelines > And for consistency I recommend to add > BuildRequires: ruby(abi) = 1.8 > > - As you have already defined %geminstdir, please use it also at > %files and so on > > - %geminstdir/[A-Z]* should be marked as %doc Will fix. These four (and maybe others) will be common for most other rubygem packages I have submitted today. I'm wondering if it would make sense if I copied this to other reviews; or what can I do to prevent duplicate reviewer work. Also, this was all generated by gem2rpm, we probably should fix the tool as well. > - Please check if Rakefile or install.rb are needed for binary rpm. Will do. > --------------------------------------------------------------------- > # Examples are documentation > mv $RPM_BUILD_ROOT%{gemdir}/gems/%{gemname}-%{version}/examples \ > $RPM_BUILD_ROOT%{gemdir}/doc/%{gemname}-%{version} > --------------------------------------------------------------------- > - Note that > --------------------------------------------------------------------- > $ gem contents term-ansicolor > --------------------------------------------------------------------- > expects that examples/ directory should be under %geminstdir. > While we allow (don't forbid) to delete some files listed in > "$ gem contents <gemname>") if packagers think they are not needed, > I don't think moving examples/ directory under %gemdir/doc is > needed. Will revert. This applies to more packages I submitted today as well. -- 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review