First package, sponsor required. Spec URL: http://marcbradshaw.co.uk/packages/SPECS/perl-Time-Duration.spec SRPM URL: http://marcbradshaw.co.uk/packages/SRPMS/perl-Time-Duration-1.04-2.fc6.src.rpm Description: This module provides functions for expressing durations in rounded or exact terms.
Heya -- a couple initial comments, not a review. Overall the spec looks fairly clean, especially for a first submission :) There are some tweaks that could be made -- and some perl-specific bits. Note you can find the (draft!) perl packaging best practices at: http://fedoraproject.org/wiki/PackagingDrafts/Perl For perl buildrequires, it's preferable to specify the module needed rather than the package containing the module (e.g. perl(Test::Pod) rather than perl-Test-Pod). There's a perl-split which may be coming down the pike. It's recommended to buildrequire Test::More or ExtUtils::MakeMaker if either are required (and it looks like the latter is). As it's a noarch package, this line can be nixed safely: find %{buildroot} -type f -name '*.bs' -size 0 -exec rm -f {} \; "chmod -R u+rwX,go+rX,go-w" works, but %{_fixperms} is so much easier on the eyes :) %doc in %files is (by convention) placed at the top of the files list. %{?dist} is usually left out of the changelog version-release. %{perl_vendorlib}/Time/Duration.pm is redundant; the first line will include %{perl_vendorlib}/Time and everything below it, as it's a directory: %{perl_vendorlib}/Time %{perl_vendorlib}/Time/Duration.pm The spec is missing a %check section; I haven't checked Time-Duration itself but if the distribution includes any tests it's important to run them. e.g. %check make test
Chris, thanks for the comments and the link. Spec file updated. New SRPM URL: http://marcbradshaw.co.uk/packages/SRPMS/perl-Time-Duration-1.04-3.fc6.src.rpm
New SRPM: http://marcbradshaw.co.uk/packages/SRPMS/perl-Time-Duration-1.04-3.src.rpm
*** Bug 243733 has been marked as a duplicate of this bug. ***
Marc, I'm willing to comaintain once you get this package approved and get sponsored (unfortunately I can't sponsor).
I can't sponsor either, but I had a look, and it looks good to me: Review for release 3: * RPM name is OK * Source Time-Duration-1.04.tar.gz is the same as upstream * Builds fine in mock * rpmlint looks OK * File list looks OK * package contains tests and they run OK * package has sane Requires and Provides Just a suggestion, it may be nice to include the ChangeLog file in %doc
Ruben: Good point, ChangeLog should indeed be in %doc, it has been added. SRPM: http://marcbradshaw.co.uk/packages/SRPMS/perl-Time-Duration-1.04-4.src.rpm Jeffrey: I would be happy to have a co-maintainer. Thanks.
Marc, have you done any pre-reviews of other packages? Basically you do everything that you would normally do for a review except that you can't actually approve the package. Doing pre-reviews will show potential sponsors that you have a good grasp of the packaging guidelines.
I do try and contribute where possible. Updated URLs http://marcbradshaw.co.uk/packages/perl-Time-Duration-1.04-4.src.rpm http://marcbradshaw.co.uk/packages/perl-Time-Duration.spec
I've done a full review, here is the result: Must Fix: --------- * Change license field to: "GPLv2+ or Artistic 2.0" Should Fix: ----------- * Fedora's default %defattr is: "%defattr(-,root,root,-) * There is a new upstream 1.06 release, this is a should Fix, as the changes seem pretty much uninteresting
All fixed. http://marcbradshaw.co.uk/packages/perl-Time-Duration-1.06-1.src.rpm
Remove the OPTIMIZE from %build. This is a noarch package, so passing CFLAGS doesn't make sense.
Approved, about the OPTIMIZE this is indeed unneeded, but not harmfull, you could consider removing this before import. Again, please wait with posting a CVS admin request until we're done with the other 2 and I've sponsored you.
Thanks. I will remove the optimize before cvs.
New Package CVS Request ======================= Package Name: perl-Time-Duration Short Description: rounded or exact English expression of durations Owners: deebs Branches: F-7 InitialCC: Cvsextras Commits: yes
cvs done.
Thanks to all for the review and thanks to Hans for the sponsorship. Jeffrey, if you would still like to co-maintain this package let me know.
Package Change Request ====================== Package Name: perl-Time-Duration New Branches: EL-4 EL-5
Package Change Request ====================== Package Name: perl-Time-Duration InitialCC: perl-sig
I think I've done what you need, if not, let me know.
No, You have replaced the owner instead of adding perl-sig into CC role. Please revert ownership to mmaslano and add perl-sig with watch* rights only.
Fixed.
Package Change Request ====================== Package Name: perl-Time-Duration New Branches: epel7 Owners: lkundrak Unfortunately both Fedora and EL-6 maintainers are not willing to maintain EPEL packages as per http://fedoraproject.org/wiki/EPEL/ContributorStatusNo
Git done (by process-git-requests).