Spec Name or Url: http://www.chrisgrau.com/packages/perl/perl-Time-Piece-MySQL.spec SRPM Name or Url: http://www.chrisgrau.com/packages/perl/perl-Time-Piece-MySQL-0.05-1.src.rpm Description: The Time::Piece::MySQL module can be used instead of, or in addition to, Time::Piece to add MySQL-specific date-time methods to Time::Piece objects.
Review: - rpmlint clean - package and spec naming OK - package meets guidelines - license is same as perl, correct in spec - spec file written in English and legible - sources match upstream - package build ok in mock on FC4 (i386) - no locales, libraries, subpackages, pkgconfigs etc. to worry about - not relocatable - no permissions issues - no duplicate files - %clean section present and correct - macro usage is consistent - code not content - no large docs - docs don't affect runtime - no scriptlets Needswork: - directory ownership wrong; perl(Time::Piece) owns %{perl_vendorarch)/Time/Piece/ not %{perl_vendorlib}/Time/Piece/ So you need to replace in %files: # Time and Time/Piece owned by perl-Time-Piece. %{perl_vendorlib}/Time/Piece/MySQL.pm with: %{perl_vendorlib}/Time/ Nitpick: - BR: perl is redundant - license text not included; suggest adding to %setup: perldoc -t perlgpl > COPYING perldoc -t perlartistic > Artistic and to %files: %doc COPYING Artistic
Created attachment 118093 [details] Patch addressing review issues
(In reply to comment #1) > - directory ownership wrong; perl(Time::Piece) owns > %{perl_vendorarch)/Time/Piece/ > not > %{perl_vendorlib}/Time/Piece/ > So you need to replace in %files: > > # Time and Time/Piece owned by perl-Time-Piece. > %{perl_vendorlib}/Time/Piece/MySQL.pm > > with: > > %{perl_vendorlib}/Time/ Oops. Can't even begin to explain what I was thinking there. Good catch. Fixed applied: http://www.chrisgrau.com/packages/perl/perl-Time-Piece-MySQL.spec http://www.chrisgrau.com/packages/perl/perl-Time-Piece-MySQL-0.05-2.src.rpm Again, I didn't use your patch, Paul, but I appreciate the effort.
Approved.