[Bug 494148] Review Request: soci - The database access library for C++ programmers

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=494148





--- Comment #3 from Denis Arnaud <denis.arnaud_fedora at m4x.org>  2009-04-05 18:48:15 EDT ---
(In reply to comment #1)
Still from the fork, I have fixed a few issues you had identified, and have
replaced the Source RPM, tar-balls and specification file.

> * For the optional Oracle Database builds, consider adding a conditional build
> parameter (see /usr/share/doc/rpm-*/conditionalbuilds), so you could simple
> build "--with oracle" to enable the optional packages instead of having to 
> edit the spec file.
Done.

> * The package %description does not mention the project name "SOCI" anywhere.
Done.

> * Package (except for the MySQL backend) doesn't adhere to the compiler flags
> guidelines.
The "configure" now reads:
%configure --disable-static \
%{?_with_mysql} %{?_without_mysql} \
%{?_with_postgresql} %{?_without_postgresql} \
%{?_with_sqlite} %{?_without_sqlite} \
%{?_with_oracle:%{?_with_oracle} %{?_with_oracle_incdir}
%{?_with_oracle_libdir}} \
%{?_without_oracle}
make %{?_smp_mflags}

It seems to work well. What did you mean by "Package does not adhere to the
compiler flag guidelines"?

> * Directory %{_includedir}/%{name}/backends/ is not included.
The main -devel package (namely, soci-devel) contains the "empty" backend, and
thus includes the %{_includedir}/%{name}/backends/empty directory. However,
since there may be other backend include directories at the building time, only
the backend/empty sub-directory should be taken in charge by the main -devel
package.

The other -devel sub-packages (namely,
soci-{mysql,postgresql,sqlite,oracle}-devel) each includes the correspnding
%{_includedir}/%{name}/backends/{mysql,postgresql,sqlite3,oracle} directory.

What did you mean by "Directory %{_includedir}/%{name}/backends/ is not
included"?

> > %package        devel
> > [...]
> > Requires(post): info
> > Requires(preun): info
> 
> "info" is not used. -devel package doesn't contain any post/preun scriptlet.  
Right. Removed.


Moreover, as said in comment #2, I'll shortly work on starting from the genuine
SOCI codebase (only adding patches) instead of starting from my "technical
fork".

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.