Bug 1128373

Summary: Review Request: perl-Audio-Cuefile-Parser - Perl cuefile parser module
Product: [Fedora] Fedora Reporter: Hans de Goede <hdegoede>
Component: Package ReviewAssignee: Christopher Meng <i>
Status: CLOSED NOTABUG QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: i, package-review, perl-devel, psabata
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2015-08-18 12:51:53 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Bug Depends On:    
Bug Blocks: 201449    

Description Hans de Goede 2014-08-09 16:58:31 UTC
Spec URL: http://jwrdegoede.danny.cz/squeezeboxserver/perl-Audio-Cuefile-Parser.spec
SRPM URL: http://jwrdegoede.danny.cz/squeezeboxserver/perl-Audio-Cuefile-Parser-0.02-1.fc21.src.rpm

Description:
Class to parse a cuefile and access the chewy, nougat centre.
Returns Audio::Cuefile::Parser::Track objects.

Fedora Account System Username: jwrdegoede

Comment 1 Christopher Meng 2014-08-11 05:28:09 UTC
1. Missing BRs of module itself:

perl(Carp)
perl(Class::Struct)
perl(IO::File)
perl(strict)
perl(warnings)

2. Missing BR of tests:

perl(File::Basename)

3. Drop this in the SPEC:

find $RPM_BUILD_ROOT -depth -type d -exec rmdir {} 2>/dev/null \;

4. Hint for f21+ do not generate packlist: perl Makefile.PL NO_PACKLIST=1

Finally, swap with bug 1128335 if you have time.

Comment 2 Hans de Goede 2014-08-12 10:42:13 UTC
Hi Christopher,

Many thanks for the review. I'm going in vacation in 2 days, and I still need to wrap up various other things, so I'm not sure if I will get around to doing a new version before then.

As for swapping, I'm all in favor of that.  bug 1128335 looks like an easy review, so I'll go and review it right away.

I've plenty more perl modules coming up. I'm trying to at least get all the modules squeezeboxserver needs which have a binary component packaged up. As these cannot be used as bundled by upstream on ARM. So when I'm back I'll happily swap some more reviews for the rest.

Note these are my first perl packages, as such it may be best if I review non perl packages for you, and I may need some guidance left and right.

perl beginner question, how did you come to the missing BuildRequires ?

Thanks,

Hans

Comment 3 Petr Šabata 2014-08-12 11:12:20 UTC
(In reply to Hans de Goede from comment #2)
> perl beginner question, how did you come to the missing BuildRequires ?

You need to go through the perl code and see what modules get actually loaded.  Look for the `use', `require' and `no' statements, although there are other ways, too...

`use' and `no' are evaluated, unlike `require', at compile time and modules loaded this way will always be needed (if your module gets actually tested).

Comment 4 Petr Šabata 2015-07-21 11:37:07 UTC
Anything new here?

Comment 5 Petr Šabata 2015-08-18 12:41:00 UTC
This looks like a dead review but I'm not allowed to reset the review flag.
Christopher, consider closing this.

Comment 6 Petr Šabata 2015-08-18 12:51:53 UTC
Actually never mind.  UI issues.