[Bug 545045] Review Request: cqrlib - ANSI C API for quaternion arithmetic and rotation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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=545045





--- Comment #2 from Takanori MATSUURA <t.matsuu@xxxxxxxxx>  2009-12-08 07:40:48 EDT ---
(In reply to comment #1)

Spec URL: http://t-matsuu.sakura.ne.jp/mock/CQRlib.spec
SRPM URL:
http://t-matsuu.sakura.ne.jp/mock/CQRlib-1.0.3-2.20090805.fc12.src.rpm

Thank you for your reviewing.

I removed the patch file because all of the modification for Makefile is now in
spec file.


> * Wrt. this construct:
> %ifarch x86_64 ppc64
> sed -e '/^LIB/s/lib/lib64/g' -i Makefile
> %endif
> 
> - Such sed calls can be generalized by applying %{_lib} (expands to "lib" on
> 32bit and to "lib64" on 64bit targets)
> 
> - Such sed calls are equivalent to "configuring" a package. They belong into
> %build

Use %{_lib} and the sed call is moved to %build section.


> * static libs
> %files devel
> ...
> %{_libdir}/*.a
> 
> Feel strongly encouraged not to ship static libs. If you really can't avoid
> shipping them, move the *.a's to a separate *-static package.

Static library is separated to static subpackage.


> * Upstream's tarball name is CQRlib. Fedora convention is to keep upstream's
> package name => Your package should be named "CQRlib", not cqrlib.

Renamed to CQRlib.


> * Your patch adds changes the shared libs from --shared to --export-dynamic and
> removes --release - Why? This is a significant behavioral change to upstream, I
> don't see any sense in.  

Probably "--shared" you said means "-dynamic".
I found that "-dynamic" should be not "--export-dynamic" but "-rdynamic".  So I
fix it to use "-rdynamic".

For "--release" issue, I keep the option in Makefile

-- 
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.

_______________________________________________
Fedora-package-review mailing list
Fedora-package-review@xxxxxxxxxx
http://www.redhat.com/mailman/listinfo/fedora-package-review

[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]