Spec URL: http://mcpierce.fedorapeople.org/rubygem-tlsmail.spec SRPM URL: http://mcpierce.fedorapeople.org/rubygem-tlsmail-0.0.1-1.fc10.src.rpm Description: This library enables pop or smtp via ssl/tls by dynamically replacing these classes to these in ruby 1.9.
I will review this. Instead I would appreciate it if you would review my review request for rubygem-nokogiri (bug 477883)
For 0.0.1-1: * Documents - Would you check if Rakefile or Manifest.txt really needed? - README.txt should be marked as %doc. * Possibly duplicate/conflicting files with system ones - This rpm contains net/pop.rb, net/smtp.rb. These are already included in ruby-libs rpm (however the files in ruby-libs seems older than those in this rpm). Can these files safely installed in parallel? * CRLF line terminators - Some ruby scripts in this rpm have CRLF line terminators (although my rpmlint could not detect them...)
Oops.. * ruby abi requirement - Also ruby(abi) requirement is needed. https://fedoraproject.org/wiki/Packaging/Ruby#Ruby_Packaging_Guidelines
(In reply to comment #2) > * Possibly duplicate/conflicting files with system ones > - This rpm contains net/pop.rb, net/smtp.rb. These are > already included in ruby-libs rpm (however the files > in ruby-libs seems older than those in this rpm). > Can these files safely installed in parallel? That is the point of this gem - it's providing alternatives to those classes in Ruby. But it only overrides them if you explicitly require the tlsmail gem in your code.
Updated with requested changes: Spec URL: http://mcpierce.fedorapeople.org/rpms/rubygem-tlsmail.spec SRPM URL: http://mcpierce.fedorapeople.org/rpms/rubygem-tlsmail-0.0.1-2.fc10.src.rpm
Well, there are still many files in binary rpm which have CRLF line terminators as $ rpm -ql rubygem-tlsmail | xargs file | grep CRLF shows (although rpmlint does not seem to be able to detect these)
(In reply to comment #6) > Well, there are still many files in binary rpm which have > CRLF line terminators as > > $ rpm -ql rubygem-tlsmail | xargs file | grep CRLF > > shows (although rpmlint does not seem to be able to detect these) Here's an update with those CRLF issues fixed: Spec URL: http://mcpierce.fedorapeople.org/rpms/rubygem-tlsmail.spec SRPM URL: http://mcpierce.fedorapeople.org/rpms/rubygem-tlsmail-0.0.1-3.fc10.src.rpm
Well, sorry for not pointing out before, however: - now you defined %gemdocsdir, it is better that you use this macro also on %files - Also you use %{gemdir}/gems/%{gemname}-%{version}/ in %files, while %{geminstdir} is also used in %files. - Now build.log warns about: ----------------------------------------------------------- warning: File listed twice: /usr/lib/ruby/gems/1.8/gems/tlsmail-0.0.1/README.txt ----------------------------------------------------------- - In %changelog, please use %% instead of single % (i.e. %%doc, for example) to aviod macros from being expanded.
(In reply to comment #8) > Well, sorry for not pointing out before, however: > > - now you defined %gemdocsdir, it is better that you use this macro > also on %files > > - Also you use %{gemdir}/gems/%{gemname}-%{version}/ in %files, while > %{geminstdir} is also used in %files. > > - Now build.log warns about: > ----------------------------------------------------------- > warning: File listed twice: > /usr/lib/ruby/gems/1.8/gems/tlsmail-0.0.1/README.txt > ----------------------------------------------------------- > > - In %changelog, please use %% instead of single % (i.e. %%doc, for > example) to aviod macros from being expanded. The above points have been fixed: SPEC URL: http://mcpierce.fedorapeople.org/rpms/rubygem-tlsmail.spec SRPM URL: http://mcpierce.fedorapeople.org/rpms/rubygem-tlsmail-0.0.1-4.fc10.src.rpm
It seems you reversed the following? (In reply to comment #2) > For 0.0.1-1: > > * Documents > - README.txt should be marked as %doc. What I mean is that %files should be ------------------------------------ %dir %{geminstdir} %doc %{geminstdir}/[A-Z]* %{geminstdir}/*/ ------------------------------------ or so. Other things are okay. ------------------------------------------------------------- This package (rubygem-tlsmail) is APPROVED by mtasaka -------------------------------------------------------------
(In reply to comment #10) > It seems you reversed the following? > > (In reply to comment #2) > > For 0.0.1-1: > > > > * Documents > > - README.txt should be marked as %doc. > > What I mean is that %files should be > ------------------------------------ > %dir %{geminstdir} > %doc %{geminstdir}/[A-Z]* > %{geminstdir}/*/ > ------------------------------------ > or so. > > Other things are okay. > ------------------------------------------------------------- > This package (rubygem-tlsmail) is APPROVED by mtasaka > ------------------------------------------------------------- Thanks. I've changed the RPM per your final suggestions. Thank you, and I'll begin your review tomorro.
New Package CVS Request ======================= Package Name: rubygem-tlsmail Short Description: This library enables pop or smtp via ssl/tls Owners: mcpierce Branches: F-9 F-10 InitialCC:
cvs done.
Closing.