Bug 471522 - Review Request: otl - OTL library for database connections and queries
Review Request: otl - OTL library for database connections and queries
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Peter Lemenkov
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-11-14 00:01 EST by Hayden James
Modified: 2008-12-02 20:27 EST (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-11-22 08:33:53 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
lemenkov: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Hayden James 2008-11-14 00:01:28 EST
Spec URL: http://hayden.doesntexist.com/~hjames/otl.spec
SRPM URL: http://hayden.doesntexist.com/~hjames/otl-devel-4.0.176-2.fc9.src.rpm
Description: OTL C++ database library/wrapper for Oracle/OCI, ODBC, and DB2.
Comment 1 Peter Lemenkov 2008-11-14 05:57:48 EST
I'll review it.
Comment 2 Peter Lemenkov 2008-11-14 06:12:17 EST
Notes: 

* You forgot to add %prep section to your spec-file (where rpmbuild should unzip sources). Please add 

%prep
%setup -q -c


This means that rpmbuild will create default directory in BUILD (%{name}-%{version}), cd into it and will quietly unzip %{SOURCE0}

* Add empty %build section (just to make rpmlint happy)

* You should use mighty power of 'install' command instead of creating directory my hands :). E.g. instead of

mkdir -p $RPM_BUILD_ROOT%{_includedir}/otl
cp -a otlv4.h $RPM_BUILD_ROOT%{_includedir}/otl

you may use

install -D -p -m 644 otlv4.h $RPM_BUILD_ROOT%{_includedir}/%{name}/otlv4.h

* Please split %description in shorter lines (to silent rpmlint):

otl.src: E: description-line-too-long OTL 4.0 was designed as a combination of a C++ template framework and OTL-adapters.
otl.src: E: description-line-too-long The framework is a generic implementation of the concept of OTL streams. The OTL-adapters
otl.src: E: description-line-too-long are thin wrappers around the database APIs and are used as class type parameters

Other things looks sane.
Comment 4 Peter Lemenkov 2008-11-15 05:10:15 EST
Few additional remarks:

* Add newline between %setup and %build (just cosmetic)
* About renaming to otl-devel - although it's not a blocker I advise you to rename it back to otl. First, if some Fedora user will decide to play with otl, his first attempt will be "yum install otl". E.g. I strongly vote against naming of packages which differs from upstream ones w/o reasons. I can't find any reasons in your case.

However some packages does prefer your current naming scheme.

If you'll finally decide to stay with otl-devel, you should add "Provides: otl" to your spec-file.

Please consider these two advices (of course, you may reject both - they're not a blocker issues) and I'll make a review.
Comment 6 Peter Lemenkov 2008-11-15 12:02:16 EST
REVIEW:

+ rpmlint is (almost) silent:

[petro@Sulaco SPECS]$ rpmlint ../RPMS/noarch/otl-devel-4.0.176-4.fc9.noarch.rpm 
otl-devel.noarch: W: no-documentation
1 packages and 0 specfiles checked; 0 errors, 1 warnings.
[petro@Sulaco SPECS]$ rpmlint ../SRPMS/otl-devel-4.0.176-4.fc9.src.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
[petro@Sulaco SPECS]$

+ The package is named according to the Package Naming Guidelines .
+ The spec file name matches the base package %{name}, in the format %{name}.spec.
+ The package meets the Packaging Guidelines .
+ The package is licensed with a Fedora approved license and meets the Licensing Guidelines .
+ The License field in the package spec file matches the actual license (BSD).
+ The spec file is written in American English.
+ The spec file for the package is legible.
+ The sources used to build the package matches the upstream source, as provided in the spec URL.

[petro@Sulaco SOURCES]$ md5sum otlv4_h.zip*
16b07c774b737bd9fa0d8e0d3a569c67  otlv4_h.zip
16b07c774b737bd9fa0d8e0d3a569c67  otlv4_h.zip.from_srpm
[petro@Sulaco SOURCES]$

+ The package successfully compiles and build into binary rpms on at least one supported architecture (ppc).
+ No additional build dependencies
+ No need to handle locales
+ Does not contain shared library files
+ The package owns all directories that it creates.
+ The package does not contain any duplicate files in the %files listing.
+ Permissions on files are set properly.
+ The package has a %clean section, which contains rm -rf $RPM_BUILD_ROOT .
+ The package consistently uses macros, as described in the macros section of Packaging Guidelines .
+ The package contains code, or permissable content.
+ Does not contain large documentation files
+ Does not contain %doc files
+ Header files are in a -devel package.
+ Does not contain static libraries
+ Does not contain pkgconfig(.pc) files
+ Does not contain  library files with a suffix
+ Does not contain any .la libtool archives
+ No a GUI application
+ The package does not own files or directories already owned by other packages.
- At the beginning of %install, the package runs rm -rf  $RPM_BUILD_ROOT
+ All filenames in rpm packages are valid UTF-8.

APPROVED.
Comment 7 Mamoru TASAKA 2008-11-15 12:55:23 EST
(removing NEEDSPONSOR)
Comment 8 Hayden James 2008-11-15 16:38:27 EST
New Package CVS Request
=======================
Package Name: otl
Short Description: OTL is a C++ template library for Oracle/OCI, ODBC, and DB2/CLI connectivity
Owners: hjames
Branches: F-9 F-10
InitialCC: mtasaka lemenkov
Comment 9 Kevin Fenzi 2008-11-16 15:09:03 EST
What is the package name here? I see otl in the request, but the reviewed package was otl-devel? 

I don't see why it would be otl-devel if upstream is named otl... 
can you explain?
Comment 10 Hayden James 2008-11-16 17:49:12 EST
This package is just for development of the OTL library. http://otl.sourceforge.net.  It was suggested that since it was a header only project that it would be more appropriate to name the package otl-devel, however I don't have any strong feelings either way, what do you think is the correct way?
Comment 11 Kevin Fenzi 2008-11-18 20:49:26 EST
Well, the upstream project is "otl" so I would call it that here as well. 
Do other linux distros ship it as otl-devel? Is it better known by that name?
Comment 12 Hayden James 2008-11-18 22:58:24 EST
I renamed the package to otl

http://hayden.doesntexist.com/~hjames/otl-4.0.176-5.fc9.src.rpm
http://hayden.doesntexist.com/~hjames/otl.spec

There are no other changes. So I would like to go ahead with the cvs request:

New Package CVS Request
=======================
Package Name: otl
Short Description: OTL is a C++ template library for Oracle/OCI, ODBC, and
DB2/CLI connectivity
Owners: hjames
Branches: F-9 F-10
InitialCC: mtasaka lemenkov


Thanks.
Comment 13 Kevin Fenzi 2008-11-19 16:37:35 EST
cvs done with the exception that user lemenkov doesn't seem to exist. ;(
Comment 14 Fedora Update System 2008-11-19 23:44:05 EST
otl-4.0.176-5.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/otl-4.0.176-5.fc9
Comment 15 Fedora Update System 2008-11-19 23:44:38 EST
otl-4.0.176-5.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/otl-4.0.176-5.fc10
Comment 16 Fedora Update System 2008-11-21 05:55:19 EST
otl-4.0.176-5.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 17 Fedora Update System 2008-12-02 20:27:49 EST
otl-4.0.176-5.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

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