Bug 1128373 - Review Request: perl-Audio-Cuefile-Parser - Perl cuefile parser module
Summary: Review Request: perl-Audio-Cuefile-Parser - Perl cuefile parser module
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Christopher Meng
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: FE-DEADREVIEW
TreeView+ depends on / blocked
 
Reported: 2014-08-09 16:58 UTC by Hans de Goede
Modified: 2015-08-18 12:51 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-08-18 12:51:53 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

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.


Note You need to log in before you can comment on or make changes to this bug.