Bug 1165625
Summary: | Review Request: perl-XML-Writer-String - Capture output from XML::Writer | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Colin Macdonald <cbm> |
Component: | Package Review | Assignee: | Petr Šabata <psabata> |
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | unspecified | ||
Version: | rawhide | CC: | package-review, psabata |
Target Milestone: | --- | Flags: | psabata:
fedora-review+
gwync: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | perl-Getopt-Tabular-0.3-2.fc21 | Doc Type: | Bug Fix |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2014-12-06 10:39:15 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | |||
Bug Blocks: | 1165620 |
Description
Colin Macdonald
2014-11-19 11:40:42 UTC
Also, builds on koji: koji.fedoraproject.org/koji/taskinfo?taskID=7754749 Hi Colin, I'll take a look at the packages you submitted and sponsor you once we get them to a passable state. Perl SIG members have enough work already; you're going to keep these :) #1 Please, link to the SPEC file itself rather than to a webpage displaying it. This link would have been much better: https://raw.githubusercontent.com/cbm755/fedora-spec/master/perl-XML-Writer-String.spec #2 Look at the package before you submit it... 5 License: CHECK(GPL+ or Artistic) #3 The current version of cpanspec generates SPEC files with lots and lots of ancient, nowadays unneeded cruft. Most of it can be removed unless you want to support old EPEL versions as well (however, not even el5 needs all of that). I will assume you're packaging only for Fedora for now. If not, we'll fix that later. So: - the BuildRoot tag can be removed - cleaning the buildroot in the %install section is not needed either - the whole %clean section can be removed - %defattr in the %files section can be removed - and you don't need to remove possible empty directories #4 Modern ExtUtils::MakeMaker understands DESTDIR so you can use that instead of PERL_INSTALL_ROOT. If you're packaging only for f21+, you can also set NO_PACKLIST which will supress installation of .packlist files and therefore you won't need to remove them in %install. NO_PACKLIST requires perl(ExtUtils::MakeMaker) >= 6.76. In the end you would get something like: %build perl Makefile.PL INSTALLDIR=vendor NO_PACKLIST=1 make %{?_smp_mflags} %install make pure_install DESTDIR=$RPM_BUILD_ROOT %{_fixperms} $RPM_BUILD_ROOT/* #5 Now the license. It is indeed "GPL+ or Artistic" ("same as Perl"), so remove the "CHECK" from the License tag. #6 The packaged files use CRLF line endings. Convert them to Unix-style. There are various ways to do this. You choose. #7 Consider packaging the `example' directory as documentation. #8 There are some missing build-time dependencies: perl, perl(strict), perl(Test) and perl(warnings). perl is called in the SPEC file and the perl modules are used by the code at %check phase. Just because it builds now doesn't mean it will tomorrow. Listing all actual dependencies is a good practice. #9 Incorrect possesive form of "it" in %description, s/it's/its/. Ok, updated: https://raw.githubusercontent.com/cbm755/fedora-spec/master/perl-XML-Writer-String.spec http://people.maths.ox.ac.uk/macdonald/fedora/perl-XML-Writer-String-0.1-4.fc21.src.rpm (I assume I should provide new SRPMs each iteration?) If you prefer, I can (try to) apply what I've learned here to the other proposals before your reviews. (In reply to Colin Macdonald from comment #4) > Ok, updated: > > https://raw.githubusercontent.com/cbm755/fedora-spec/master/perl-XML-Writer- > String.spec > > http://people.maths.ox.ac.uk/macdonald/fedora/perl-XML-Writer-String-0.1-4. > fc21.src.rpm Hmm, I see the SRPM has a bumped release but the SPEC is still the same right now. > (I assume I should provide new SRPMs each iteration?) Yes, you should. > If you prefer, I can (try to) apply what I've learned here to the other > proposals before your reviews. That would be great. I posted some comments elsewhere, too. I'll give you some time and check the rest tomorrow or so. "Hmm, I see the SRPM has a bumped release but the SPEC is still the same right now." sorry, pushed now. Ok, much better. Just please change BuildRequires: perl(:MODULE_COMPAT_%(eval "`perl -V:version`"; echo $version)) to BuildRequires: perl Here's an explanation what MODULE_COMPAT is and why we use it: https://fedoraproject.org/wiki/Packaging:Perl#Versioned_MODULE_COMPAT_Requires You also added a runtime dependency on XML::Writer. Although it's not technically needed, despite what Makefile.PL and README say, I agree it's good to have it. thanks, addressed the BR: http://people.maths.ox.ac.uk/macdonald/fedora/perl-XML-Writer-String-0.1-5.fc21.src.rpm https://raw.githubusercontent.com/cbm755/fedora-spec/master/perl-XML-Writer-String.spec Ack, approving. New Package SCM Request ======================= Package Name: perl-XML-Writer-String Short Description: Capture output from XML::Writer module Upstream URL: http://search.cpan.org/dist/XML-Writer-String/ Owners: cbm mef Branches: f21 InitialCC: perl-sig Git done (by process-git-requests). perl-Getopt-Tabular-0.3-2.fc21,perl-Business-ISSN-0.91-4.fc21,perl-Tie-Cycle-1.20-2.fc21,perl-XML-LibXML-Simple-0.94-3.fc21,perl-XML-Writer-String-0.1-5.fc21,perl-ExtUtils-LibBuilder-0.06-4.fc21 has been submitted as an update for Fedora 21. https://admin.fedoraproject.org/updates/perl-Getopt-Tabular-0.3-2.fc21,perl-Business-ISSN-0.91-4.fc21,perl-Tie-Cycle-1.20-2.fc21,perl-XML-LibXML-Simple-0.94-3.fc21,perl-XML-Writer-String-0.1-5.fc21,perl-ExtUtils-LibBuilder-0.06-4.fc21 (In reply to Fedora Update System from comment #12) > perl-Getopt-Tabular-0.3-2.fc21,perl-Business-ISSN-0.91-4.fc21,perl-Tie-Cycle- > 1.20-2.fc21,perl-XML-LibXML-Simple-0.94-3.fc21,perl-XML-Writer-String-0.1-5. > fc21,perl-ExtUtils-LibBuilder-0.06-4.fc21 has been submitted as an update > for Fedora 21. > https://admin.fedoraproject.org/updates/perl-Getopt-Tabular-0.3-2.fc21,perl- > Business-ISSN-0.91-4.fc21,perl-Tie-Cycle-1.20-2.fc21,perl-XML-LibXML-Simple- > 0.94-3.fc21,perl-XML-Writer-String-0.1-5.fc21,perl-ExtUtils-LibBuilder-0.06- > 4.fc21 This update looks more packed than it needs to be. Leave it for now but, please, next time submit each update independently, if possible. > next time submit each update independently, if possible.
Ok, will do Now do I just wait for karma right?
On the topic of F21: for packages that depend on that update (e.g., perl-Business-ISMN, perl-TextBibTeX): should I wait for the update to pass or is it better to request a buildroot override? (In theory, I can just update them one-at-a-time.)
(In reply to Colin Macdonald from comment #14) > > next time submit each update independently, if possible. > > Ok, will do Now do I just wait for karma right? Indeed. Or until some time passes. It's usually the latter. For Fedora 21 it should be three days right now. > On the topic of F21: for packages that depend on that update (e.g., > perl-Business-ISMN, perl-TextBibTeX): should I wait for the update to pass > or is it better to request a buildroot override? (In theory, I can just > update them one-at-a-time.) Either is fine. A buildroot override will allow you to get them all to stable faster. Just make sure you don't push these packages to stable before their deps land there. (which is actually what updates with multiple packages in them are for -- you create an override with your dependency and then issue an update containing both that dependency and the dependant package or packages). perl-Getopt-Tabular-0.3-2.fc21, perl-Business-ISSN-0.91-4.fc21, perl-Tie-Cycle-1.20-2.fc21, perl-XML-LibXML-Simple-0.94-3.fc21, perl-XML-Writer-String-0.1-5.fc21, perl-ExtUtils-LibBuilder-0.06-4.fc21 has been pushed to the Fedora 21 testing repository. perl-Getopt-Tabular-0.3-2.fc21, perl-Business-ISSN-0.91-4.fc21, perl-Tie-Cycle-1.20-2.fc21, perl-XML-LibXML-Simple-0.94-3.fc21, perl-XML-Writer-String-0.1-5.fc21, perl-ExtUtils-LibBuilder-0.06-4.fc21 has been pushed to the Fedora 21 stable repository. |