Spec URL: http://nboyle.fedorapeople.org/perl-RPC-XML.spec SRPM URL: http://nboyle.fedorapeople.org/perl-RPC-XML-0.59-2.fc8.src.rpm Description: The RPC::XML package is an implementation of XML-RPC. The module provides classes for sample client and server implementations, a server designed as an Apache location-handler, and a suite of data-manipulation classes that are used by them.
FYI - License is as follows: ##################################################################### Copying and distribution are permitted under the terms of the Artistic License as distributed with Perl versions 5.005 and later. See http://www.opensource.org/licenses/artistic-license.php ##################################################################### I'm interpreting the "and later" to mean Artistic 2.0, as opposed to 1.0, the latter of which is unacceptable as per the guidelines. If I'm incorrect in my legalese, let me know, and I'll ping upstream to clarify license, and/or re-license.
I think "and later" refers to the perl version. It would definitely be good to get clarification from the upstream developer. It's sufficient to simply get an email from them which you include in the package.
Alright I got license clarification from upstream. New files: Spec URL: http://nboyle.fedorapeople.org/perl-RPC-XML.spec SRPM URL: http://nboyle.fedorapeople.org/perl-RPC-XML-0.59-3.fc8.src.rpm
A couple thoughts prior to a full-fledged review: The %perl_vendorarch and %perl_vendorlib macros have been defined by the system since... oh, about RHEL3 or so. They should probably be dropped unless you're explicitly keeping them in there to better deal with old systems; in that case they should be conditionalized, however. Using macros a la %{__rm} is legit, and you use them cleanly and consistently, but they're still a bit odd in Fedora. I wouldn't consider it a blocker, however. :) Your buildroot is slightly wonky; Fedora generally uses %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) You have the man pages marked %doc... AFAIK convention is to not mark these as such, though I'm not entirely sure convention is correct in this case. The Apache::RPC::Server/::Status modules pull in mod_perl, which in turn has a slew of requirements of its own (not the least of which is apache). I'd break them out into a subpackage to avoid a rather large potentially unwanted dependency tree. Your buildrequires are incomplete: judging by Makefile.PL there are a couple more you're going to want to throw in there. Note also that it's key to start including core modules such as Test::More if so required. It's often helpful to kick off a koji scratch build and post the link to the task results in the bug review (a relatively recent development). It's also a good self-sanity check to ensure the package builds as intended.
(In reply to comment #4) > The %perl_vendorarch and %perl_vendorlib macros have been defined by the > system since... oh, about RHEL3 or so. They should probably be dropped Done. > Your buildroot is slightly wonky; Fedora generally uses > %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) The method I'm using is preferred as per the wiki; see: http://fedoraproject.org/wiki/Packaging/Guidelines#head-b4fdd45fa76cbf54c885ef0836361319ab962473 > You have the man pages marked %doc... AFAIK convention is to not mark these as > such, though I'm not entirely sure convention is correct in this case. You're right, fixed > The Apache::RPC::Server/::Status modules pull in mod_perl, which in turn has a > slew of requirements of its own (not the least of which is apache). I'd break > them out into a subpackage to avoid a rather large potentially unwanted > dependency tree. Agreed... I wasn't sure of an appropriate name, so for now it's perl-Apache-RPC. If there's another way to name it that makes more sense, let me know! :) > Your buildrequires are incomplete: judging by Makefile.PL there are a couple > more you're going to want to throw in there. Note also that it's key to start > including core modules such as Test::More if so required. There is a warning about a missing XML::LibXML, but only to inform the user this will be required under RPC-XML v0.60. Other modules required in Makefile.PL, that aren't in the spec, are part of the core perl package (ex: Cwd, and File::Find). > It's often helpful to kick off a koji scratch build and post the link to the > task results in the bug review (a relatively recent development). It's also a > good self-sanity check to ensure the package builds as intended. This mock builds on 8 and devel i386 for me, without issue. Additionally, it scratch built in Koji for me: F8: http://koji.fedoraproject.org/koji/taskinfo?taskID=518717 F9: http://koji.fedoraproject.org/koji/taskinfo?taskID=518727 Is there something specific you noticed in building that I'm overlooking? At any rate, the package/subpackage version is now available here: Spec URL: http://nboyle.fedorapeople.org/perl-RPC-XML.spec SRPM URL: http://nboyle.fedorapeople.org/perl-RPC-XML-0.59-4.fc8.src.rpm I've never really worked with the whole subpackage business, so please look over that with extra scrutiny ;)
Some additional comments: There's a test suite included; any reason why it's not being run? Of course, when you do run it, you'll find out why Chris was telling you about missing build dependencies, because most of the tests will fail without Test::More and XML::Parser. Note that pretty much any reviewer will consider it a blocker to fail to run the test suite without a good reason. Also, is there any particular reason why you didn't just use cpanspec to generate the package? While there's nothing specifically wrong with rolling your own by hand, it's a good but more difficult to review since it doesn't look anything like essentially all of the other Perl packages in the distro.
Sorry for the long delay in getting these issues fixed: New version: Spec URL: http://nboyle.fedorapeople.org/perl-RPC-XML.spec SRPM URL: http://nboyle.fedorapeople.org/perl-RPC-XML-0.59-5.fc8.src.rpm (In reply to comment #6) > There's a test suite included; any reason why it's not being run? Of course, > when you do run it, you'll find out why Chris was telling you about missing > build dependencies, because most of the tests will fail without Test::More and > XML::Parser. Note that pretty much any reviewer will consider it a blocker to > fail to run the test suite without a good reason. Chalk this up to my inexperience with packaging Perl modules. I didn't realize this module had a unit tests with it... I've since added both missing build dependencies. > Also, is there any particular reason why you didn't just use cpanspec to > generate the package? While there's nothing specifically wrong with rolling > your own by hand, it's a good but more difficult to review since it doesn't look > anything like essentially all of the other Perl packages in the distro. Again, inexperience (you've got to start somewhere though, right?). I wasn't even aware of such a tool. I had originally adopted this spec from Dries, although admittedly by this point it's almost a complete rewrite. My thoughts are that at this point, it's best to just leave it as-is and keep this tool in mind for future packagings. If it still makes more sense to redo it using cpanspec, I'll gladly do that. Let me know.
two minor issues: - imho you should add "-p" to the %{__cp} of Source1 to preserve the timestamp - You should add README.license to %doc of the main package, otherwise there is no need to cp it ;-) maybe one major issue: - I guess you need to add the testsuite to %check in the spec, but I do not really know anything, because I did not read anything about testsuites in any Fedora guideline, but only in some reviews that there was no need for %check, because there was no testsuite.
(In reply to comment #8) > - I guess you need to add the testsuite to %check in the spec, but I do not > really know anything, because I did not read anything about testsuites in any > Fedora guideline, but only in some reviews that there was no need for %check, > because there was no testsuite. Test suites _must_ be run for perl packages; see: http://fedoraproject.org/wiki/Packaging/Perl#Testing_and_Test_Suites I like the splitting of the Apache bits off into their own sub-package :) The %files section should be amended so the correct package owns %{perl_vendorlib}/Apache and %{perl_vendorlib}/RPC. See http://fedoraproject.org/wiki/Packaging/Perl#Directory_Ownership Your license shortname is off, and missing the LGPL :) See http://fedoraproject.org/wiki/Licensing#Good_Licenses There appear to be some samples/docs under the etc/, ex/ and methods/ directories. It may be worthwhile to include them in %doc. Do these things, and (from my cursory scan of the spec) I believe this package will be ready for a review :)
...and 0.60 was released on 09 Apr 2008.
Ping? What's the status of this package?
Last comment from the submitter was over two months ago. Setting NEEDINFO; I will close this ticket soon if there is no response.
Closing.