Bug 462297

Summary: Review Request: perl-o2sms - A perl module to send SMS messages using .ie websites
Product: [Fedora] Fedora Reporter: Niall Sheridan <nsheridan>
Component: Package ReviewAssignee: Miroslav Suchý <msuchy>
Status: CLOSED NOTABUG QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, lemenkov, msuchy, notting
Target Milestone: ---Flags: msuchy: fedora‑review+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2009-10-02 04:39:34 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
Bug Depends On: 462296    
Bug Blocks: 201449    

Description Niall Sheridan 2008-09-15 03:44:42 EDT
Spec URL: http://www.evil.ie/fedora/rpms/o2sms/perl-o2sms.spec
SRPM URL: http://www.evil.ie/fedora/rpms/o2sms/perl-o2sms-3.29-1.fc9.src.rpm
Description:
o2sms is a program to send SMS messages using the websites of Irish mobile operators.
The program works by simulating a web browser's interaction with those
websites. This script requires a valid web account with O2 Ireland, Vodafone
Ireland or Meteor Ireland.
Comment 1 Miroslav Suchý 2009-02-17 07:45:13 EST
OK - Package meets naming and packaging guidelines
OK - Spec file matches base package name.
OK - Spec has consistant macro usage.
OK - Meets Packaging Guidelines.
OK - License
OK - License field in spec matches
OK - Spec in American English
OK - Spec is legible.
FAIL - Sources match upstream md5sum:
  could not find upstream tar.gz on given url (404 not found)
N/A - Package needs ExcludeArch
OK - BuildRequires correct
N/A - Spec handles locales/find_lang
N/A - Package is relocatable and has a reason to be.
OK - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
OK - Package has correct buildroot
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
OK - Package is code or permissible content.
OK - Doc subpackage needed/used.
N/A - Headers/static libs in -devel subpackage.
N/A - Spec has needed ldconfig in post and postun
N/A - .pc files in -devel subpackage/requires pkgconfig
N/A - .so files in -devel subpackage.
N/A - -devel package Requires: %{name} = %{version}-%{release}
N/A - .la files are removed.
N/A - Package is a GUI app and has a .desktop file
OK - Package compiles and builds on at least one arch.
OK - Package has no duplicate files in %files.
FAIL - Package doesn't own any directories other packages own.
OK - Package owns all the directories it creates.
OK - No rpmlint output.
OK - final provides and requires are sane:
Provides: perl(WWW::SMS::IE::aftsms) = 288 perl(WWW::SMS::IE::iesms) = 333 perl(WWW::SMS::IE::meteorsms) = 288 perl(WWW::SMS::IE::o2sms) = 288 perl(WWW::SMS::IE::vodasms) = 312
Requires: /usr/bin/perl perl(Data::Dumper) perl(File::Basename) perl(File::Temp) perl(File::stat) perl(Getopt::Long) >= 2.33 perl(POSIX) perl(Pod::Usage) perl(Storable) perl(WWW::SMS::IE::aftsms) perl(WWW::SMS::IE::iesms) perl(WWW::SMS::IE::meteorsms) perl(WWW::SMS::IE::o2sms) perl(WWW::SMS::IE::vodasms) perl(constant) perl(strict) perl(vars) perl(warnings)


SHOULD Items:
OK, tested on x86_64 - Should build in mock.
OK - Should build on all supported archs
Didn't test - Should function as described.
No scriptlets - Should have sane scriptlets.
N/A - Should have subpackages require base package with fully versioned depend.
OK - Should have dist tag
FAIL - Should package latest version
N/A - check for outstanding bugs on package. (For core merge reviews)

TODO: 
can point SOURCE0 to cpan when old relases are kept? (mandatory)
can you update to latest version? (optional)
Comment 2 Niall Sheridan 2009-02-21 08:10:52 EST
Hi

Thanks for taking the time to review this.
I've fixed Source0 in the specfile to
Source0:        http://search.cpan.org/CPAN/authors/id/M/MA/MACKERS/o2sms-%{version}.tar.gz
I'll update to the latest version.

I have one question about "Package doesn't own any directories other packages own." - can you elaborate on this failure? I assume it refers to owning %{vendor_perl}/WWW/ - should I change this?
Comment 3 Miroslav Suchý 2009-02-23 04:08:30 EST
Mea culpa. I recently have similar issue in different packages and I find that you *should* own  the directories:
/usr/lib/perl5/vendor_perl/5.10.0/WWW
/usr/lib/perl5/vendor_perl/5.10.0/WWW/SMS
/usr/lib/perl5/vendor_perl/5.10.0/WWW/SMS/IE
Which you already own. So you had it correctly. Sorry for confusion.
So please just upload new version with updated Source0 and that will be all.
Comment 4 Niall Sheridan 2009-02-23 14:11:17 EST
Thanks for clearing that up.
I've updated the package to the latest version, tested on i386 and x86_64 and fixed Source0. Version 3.32 is at:
http://www.evil.ie/fedora/rpms/o2sms/perl-o2sms.spec
http://www.evil.ie/fedora/rpms/o2sms/perl-o2sms-3.32-1.fc10.src.rpm
Comment 5 Miroslav Suchý 2009-02-24 03:33:33 EST
md5sum is cc3b56373ba2d380b19d9142e4173ec7 for both tar.gz
package build cleanly

APPROVED
Comment 6 Miroslav Suchý 2009-03-17 02:44:06 EDT
Ping. Any reason why didn't you requested cvs branch a didn't build package yet?
Comment 7 Peter Lemenkov 2009-05-23 03:00:42 EDT
Ping again, Niall!
We all waiting for you :)
Comment 8 Peter Lemenkov 2009-09-25 09:24:17 EDT
Another one ping.
Niall, are you still here?
Comment 9 Peter Lemenkov 2009-10-02 04:39:34 EDT
Closing as FE-DEADREVIEW