Bug 1095662 - Review Request: perl-Sys-Detect-Virtualization - Detect if a UNIX system is running as a virtual machine
Summary: Review Request: perl-Sys-Detect-Virtualization - Detect if a UNIX system is r...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Petr Šabata
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 1091144
Blocks:
TreeView+ depends on / blocked
 
Reported: 2014-05-08 10:14 UTC by David Dick
Modified: 2014-06-12 23:36 UTC (History)
2 users (show)

Fixed In Version: perl-Sys-Detect-Virtualization-0.106-1.el6
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2014-06-01 00:00:40 UTC
Type: ---
Embargoed:
psabata: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description David Dick 2014-05-08 10:14:00 UTC
Spec URL: http://ddick.fedorapeople.org/packages/perl-Sys-Detect-Virtualization.spec
SRPM URL: http://ddick.fedorapeople.org/packages/perl-Sys-Detect-Virtualization-0.106-1.fc20.src.rpm
Description: Detect if a UNIX system is running as a virtual machine
Fedora Account System Username: ddick

Comment 1 Petr Šabata 2014-05-13 01:00:50 UTC
Missing BRs:
perl(Config)
perl(ExtUtils::MM_Unix)

Your package always run-requires Parse::DMIDecode; you want to remove line 41.

Are the explicit run-requires in the virtdetect package necessary?

It might not be obvious to the user that virtdetect is a subpackage of this one; maybe packaging the README in its own %doc would be a good idea?

Comment 2 David Dick 2014-05-13 08:38:52 UTC
(In reply to Petr Šabata from comment #1)
> Missing BRs:
> perl(Config)
> perl(ExtUtils::MM_Unix)

Done.

> Your package always run-requires Parse::DMIDecode; you want to remove line
> 41.

*ouch* Done.

> Are the explicit run-requires in the virtdetect package necessary?

It was the only way i knew to remove the requires for Getopt::Long, POSIX, etc from perl-Sys-Detect-Virtualization (where they are not required) and keep them in virtdetect (where they are).  Suggestions?

> It might not be obvious to the user that virtdetect is a subpackage of this
> one; maybe packaging the README in its own %doc would be a good idea?

Okay.  Along the same sort of line, i thought including the Changelog would be useful too?

Comment 3 Petr Šabata 2014-05-13 14:09:37 UTC
(In reply to David Dick from comment #2)
> (In reply to Petr Šabata from comment #1)
> > Missing BRs:
> > perl(Config)
> > perl(ExtUtils::MM_Unix)
> 
> Done.

Check.

> > Your package always run-requires Parse::DMIDecode; you want to remove line
> > 41.
> 
> *ouch* Done.

Check.

> > Are the explicit run-requires in the virtdetect package necessary?
> 
> It was the only way i knew to remove the requires for Getopt::Long, POSIX,
> etc from perl-Sys-Detect-Virtualization (where they are not required) and
> keep them in virtdetect (where they are).  Suggestions?

You don't need to remove them at all; they're not there :)
Just drop the req filters in the main package and explicit run requires in the virtdetect one.  It will work like a charm.  The dependencies (and provides) are put in the packages according to what files they include.

> > It might not be obvious to the user that virtdetect is a subpackage of this
> > one; maybe packaging the README in its own %doc would be a good idea?
> 
> Okay.  Along the same sort of line, i thought including the Changelog would
> be useful too?

Could be but I find it somewhat less important; the README points the user to the main package which is installed and provides the Changelog already.

Comment 4 David Dick 2014-05-14 07:24:28 UTC
(In reply to Petr Šabata from comment #3)
> (In reply to David Dick from comment #2)
> > (In reply to Petr Šabata from comment #1)
> > > Are the explicit run-requires in the virtdetect package necessary?
> > 
> > It was the only way i knew to remove the requires for Getopt::Long, POSIX,
> > etc from perl-Sys-Detect-Virtualization (where they are not required) and
> > keep them in virtdetect (where they are).  Suggestions?
> 
> You don't need to remove them at all; they're not there :)
> Just drop the req filters in the main package and explicit run requires in
> the virtdetect one.  It will work like a charm.  The dependencies (and
> provides) are put in the packages according to what files they include.

Hmmm... yes it does.. filters and explicit run requires removed.

> > > It might not be obvious to the user that virtdetect is a subpackage of this
> > > one; maybe packaging the README in its own %doc would be a good idea?
> > 
> > Okay.  Along the same sort of line, i thought including the Changelog would
> > be useful too?
> 
> Could be but I find it somewhat less important; the README points the user
> to the main package which is installed and provides the Changelog already.

Okay. removed Changelog file.

Comment 5 Petr Šabata 2014-05-14 09:40:08 UTC
Approving.

Comment 6 David Dick 2014-05-14 10:41:55 UTC
New Package SCM Request
=======================
Package Name: perl-Sys-Detect-Virtualization
Short Description: Detect if a UNIX system is running as a virtual machine
Owners: ddick
Branches: f20 el6 epel7
InitialCC: perl-sig

Thanks for the review Petr.

Comment 7 Gwyn Ciesla 2014-05-14 11:31:09 UTC
Git done (by process-git-requests).

Comment 8 Fedora Update System 2014-05-22 08:20:28 UTC
perl-Sys-Detect-Virtualization-0.106-1.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/perl-Sys-Detect-Virtualization-0.106-1.fc20

Comment 9 Fedora Update System 2014-05-23 18:57:32 UTC
perl-Sys-Detect-Virtualization-0.106-1.fc20 has been pushed to the Fedora 20 testing repository.

Comment 10 Fedora Update System 2014-05-28 09:12:12 UTC
perl-Sys-Detect-Virtualization-0.106-1.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/perl-Sys-Detect-Virtualization-0.106-1.el6

Comment 11 Fedora Update System 2014-06-01 00:00:40 UTC
perl-Sys-Detect-Virtualization-0.106-1.fc20 has been pushed to the Fedora 20 stable repository.

Comment 12 Fedora Update System 2014-06-12 23:36:05 UTC
perl-Sys-Detect-Virtualization-0.106-1.el6 has been pushed to the Fedora EPEL 6 stable repository.


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