Bug 244948 - Review Request: perl-Config-Std - Yet Another Way of Storing Configuration Files
Review Request: perl-Config-Std - Yet Another Way of Storing Configuration Files
Status: CLOSED NOTABUG
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Jason Tibbitts
Fedora Extras Quality Assurance
:
Depends On:
Blocks: FE-DEADREVIEW
  Show dependency treegraph
 
Reported: 2007-06-19 20:32 EDT by David Fetter
Modified: 2008-01-30 14:44 EST (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-01-30 14:44:53 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)
Patch to fix specfile issues. (1.58 KB, patch)
2007-09-27 13:00 EDT, Jason Tibbitts
no flags Details | Diff

  None (edit)
Description David Fetter 2007-06-19 20:32:30 EDT
Spec URL: http://fetter.org/rpm/perl-Config-Std.spec
SRPM URL: http://fetter.org/rpm/perl-Config-Std-v0.0.4-1.fc7.src.rpm
Description: Yet Another Way of Storing Configuration Files
Comment 1 David Fetter 2007-06-20 15:48:47 EDT
rpmlint-clean files:

SPEC: http://fetter.org/rpm/perl-Config-Std.spec
SRPM: http://fetter.org/rpm/perl-Config-Std-v0.0.4-2.fc7.src.rpm
Comment 2 Jason Tibbitts 2007-07-29 00:11:25 EDT
This fails to build for me due to missing BuildRequires:

 - ERROR: Test::More is not installed

and then all of the tests fail:

FAILED--11 test scripts could be run, alas--no output ever seen
Comment 3 David Fetter 2007-09-22 17:22:11 EDT
Used cpanspec to create new spec file and RPM

SPEC:  http://fetter.org/rpm/perl-Config-Std.spec
SRPM:  http://fetter.org/rpm/perl-Config-Std-v0.0.4-3.fc7.src.rpm
Comment 4 Jason Tibbitts 2007-09-25 01:23:06 EDT
The most recent version still doesn't have a build dependency on Test::More, so
all the tests fail and the package fails to build.

Also, this package has the same issue with the license that
perl-Business-CreditCard has.  According to the documentation, License: should
be "GPL+ or Artistic".

Adding the necessary build dependency gets things building, although for
complete test suite coverage you should also have build dependencies on
perl(Test::Pod::Coverage) and perl(Test::Pod).  I added the necessary bits so
that I could do this review.

rpmlint complains about the License: tag, and also
 perl-Config-Std.noarch: W: incoherent-version-in-changelog 0.0.4-3 v0.0.4-3.fc8
Indeed, this package has ended up with a stray 'v' in the Version which comes
from upstream.  Frankly I've no idea why upstream would do a thing like that,
but we want the Version: tag to be numeric in this case.  So you'll need to
remove the 'v' from Version: and, so that things still build, add that 'v' back
to your Source0: and %setup lines.

I know %description comes from the package itself (specifically the Description
section of the documentation) but I would urge you to remove the needless
semi-profanity.  (Not that it offends me, but we're creating a distribution to
be used by very many people.)


* source files match upstream:
   40b455d1971960514a0b87c58a4d1656207e03cd6d7a33dd3c068f97ad3ed5d5  
   Config-Std-v0.0.4.tar.gz
X package version is not compliant with guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
? description is questionable.
* dist tag is present.
* build root is OK.
X license field is incorrect.
* license is open source-compatible.
* license text not included upstream.
* latest version is being packaged.
X BuildRequires are proper (missing perl(Test::More)).
* %clean is present.
X package fails to build.
* once made to build, the package installs properly
X rpmlint has valid complaints.
* final provides and requires are sane:
   perl(Config::Std)
   perl(Config::Std::Block)
   perl(Config::Std::Comment)
   perl(Config::Std::Gap)
   perl(Config::Std::Hash)
   perl(Config::Std::Keyval)
   perl-Config-Std = v0.0.4-3.fc8
  =
   perl(:MODULE_COMPAT_5.8.8)
   perl(Carp)
   perl(Class::Std)
   perl(Fcntl)
   perl(version)

* %check is present and once necessary build dependencies are added, all tests 
  pass:
   All tests successful.
   Files=11, Tests=31,  0 wallclock secs ( 0.50 cusr +  0.20 csys =  0.70 CPU)
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no scriptlets present.
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
Comment 5 David Fetter 2007-09-25 14:31:33 EDT
I've bowdlerized the description as you requested and added a dependency on
Test::More.

SPEC:  http://fetter.org/rpm/perl-Config-Std.spec
SRPM:  http://fetter.org/rpm/perl-Config-Std-v0.0.4-4.fc7.src.rpm
Comment 6 Jason Tibbitts 2007-09-26 15:52:52 EDT
You are still missing build dependencies on perl(Test::Pod) and
perl(Test::Pod::Coverage) so you miss some test suite coverage; this changes the
test results to:
  All tests successful, 2 tests skipped.
  Files=11, Tests=29,  0 wallclock secs ( 0.42 cusr +  0.15 csys =  0.57 CPU)
It's good to have full test coverage but you don't seem to want to add those two
bbuild dependencies for whatever reason so I won't block on it.

The license field is fixed, so rpmlint is quiet.  The following issue remains:
X package version is not compliant with guidelines.
Comment 7 David Fetter 2007-09-26 17:11:38 EDT
I've added a dependency on Test::Pod but not on Test::Pod::Coverage, as it built
without, and made it rpmlint-clean, which I believe brings package version into
compliance with guidelines.

SPEC:  http://fetter.org/rpm/perl-Config-Std.spec
SRPM:  http://fetter.org/rpm/perl-Config-Std-v0.0.4-5.fc7.src.rpm
Comment 8 Jason Tibbitts 2007-09-27 12:59:29 EDT
If it still shows up with a 'v' in the version then it doesn't meet the
guidelines.  Perhaps I'm just not being clear with my descriptions of the
problems, so I'll attach a patch against your specfile and you can check out
what changes I'm talking about:

Change version to just "0.0.4".

Change Source0: to put the 'v' in so the upstream tarball can still be found.

Add BuildRequires: perl(Test::Pod::Coverage)

Change %setup macro to put the 'v' in so the untarred directory can still be found.

Fix up the changelog to match the non-'v' version.

Of course, if you apply this you'll want to bump the release and add a new
changelog entry for it.
Comment 9 Jason Tibbitts 2007-09-27 13:00:59 EDT
Created attachment 208711 [details]
Patch to fix specfile issues.
Comment 10 David Fetter 2007-09-27 13:20:42 EDT
Added Test::Pod::Coverage, changed version number correctly.

SPEC:  http://fetter.org/rpm/perl-Config-Std.spec
SRPM:  http://fetter.org/rpm/perl-Config-Std-0.0.4-6.fc7.src.rpm
Comment 11 Jason Tibbitts 2007-09-27 15:37:16 EDT
OK, rpmlint is quiet, the version looks good.  All the issues I had with this
package are fixed.

APPROVED

Now on to perl-Business-CreditCard.
Comment 12 Jason Tibbitts 2008-01-30 14:44:53 EST
Revoking approval and closing as detailed in bug 244947.

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