[Bug 753597] Review Request: yazpp - C++ API for YAZ

[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=753597

Martin Gieseking <martin.gieseking@xxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |martin.gieseking@xxxxxx

--- Comment #3 from Martin Gieseking <martin.gieseking@xxxxxx> 2011-11-20 05:33:04 EST ---
Ryan, here are a couple of initial comments:

- please always check your rpms (source AND binary packages) with rpmlint:

$ rpmlint ./yazpp*.rpm
yazpp.x86_64: E: devel-dependency libyaz-devel
yazpp.x86_64: E: library-without-ldconfig-postin /usr/lib64/libyazpp.so.4.0.0
yazpp.x86_64: E: library-without-ldconfig-postun /usr/lib64/libyazpp.so.4.0.0
yazpp.x86_64: E: library-without-ldconfig-postin /usr/lib64/libzoompp.so.4.0.0
yazpp.x86_64: E: library-without-ldconfig-postun /usr/lib64/libzoompp.so.4.0.0
4 packages and 0 specfiles checked; 5 errors, 0 warnings.


- the Summary should be a bit more verbose ;)

- change the Group of the base package to "System Environment/Libraries"

- if you want to build the package for EPEL < 6 as well, adapt the BuildRoot
  field according to 
  https://fedoraproject.org/wiki/EPEL/GuidelinesAndPolicies#BuildRoot_tag

  If you targeting Fedora and/or EPEL 6 only, you can drop the BuildRoot tag,
  the %clean section, the initial cleaning of the buildroot in %install, and
  all %defattr lines

- Don't mix $RPM_BUILD_ROOT and %{buildroot}. Choose one variant and stick 
  with it.

- Why does the base package require libyaz-devel? I think you can drop it.

- replace "%package -n  yazpp-devel" with "%package devel", and 
  "%description -n  yazpp-devel" with "%description devel"
  Same for %files:
  replace "%files -n yazpp" with %files, and 
  "%files -n yazpp-devel" with "%files devel"

- if you want to keep the %defattr lines, change them to 
  %defattr(-,root,root,-)

- .la files must not be packaged in Fedora. Remove it in %install and drop it
  from %files

- Static libraries must go to a separate -static package. However, Fedora
  usually doesn't ship static libraries. If there's no good reason for 
  providing it, just add --disable-static to %configure, and remove the .a 
  file from %files.

- add "%post -p /sbin/ldconfig" and "%postun -p /sbin/ldconfig" between 
  %clean and %file section:
  https://fedoraproject.org/wiki/Packaging/Guidelines#Shared_Libraries

- remove the echo line from the spec

- the devel package must require the base package this way:
  https://fedoraproject.org/wiki/Packaging/Guidelines#Requiring_Base_Package

- Please be more specific in %files, especially if you add only few files.
  This helps to avoid adding unwanted files and to get a quick overview of
  what's actually put into the package. For example, instead of  
  %{_libdir}/*.so.* it's better to write:
  %{_libdir}/libyazpp.so.*
  %{_libdir}/libzoompp.so.*

- add file ChangeLog to the %docs

- The tarball contains a Doxyfile, so it's probably a good idea to build the
  doxygen documentation and add it to the %docs. As there is quite a lot of
  documentation available, I suggest to put it in a -doc subpackage.

- yazzpp provides some command-line tools, too (yaz-my-client, yaz-my-server, 
  zclient, zlint). Maybe you should add them to a -utils subpackage.

-- 
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.
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review



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