[Bug 459281] Review Request: corosync - The Corosync Cluster Engine

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


Jason Tibbitts <tibbs at math.uh.edu> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |tibbs at math.uh.edu




--- Comment #2 from Jason Tibbitts <tibbs at math.uh.edu>  2008-08-15 20:38:03 EDT ---
Any particular reason this requires a fixed uid?

Why call %{_initrddir}/corosync directly instead of using /sbin/service
corosync?

Also note that if you're targeting F10 and later, you can use _initddir.  I'm
not sure anyone knows where that extra 'r' came from.

What is a ".lcrso" file, and why do they need to be under %{_libexecdir}?  Have
you investigated how this works with multilib?

You shouldn't need to explicitly mark anything under %{_docdir} as %doc.  In
fact, you don't usually install doc files in %doc, but just list them as %doc
in the %files list and they'll be copied there.  I.e. you can just do
  %doc LICENSE SECURITY

Are you sure the libraries should be in the -devel package?  The unversioned
.so files should go there, definitely, but the actual libraries should
generally be in the main package.  Otherwise, in order to run something built
against them you have to install the -devel package, which is rarely correct. 

Why is corosync-objctl installed in the main package while its manpage is in
the -devel package?  I didn't read the other manpages, but if you're sure that
they're of use only to developers then they're fine in the -devel package.

You could probably make your %files lists very much smaller if you didn't list
each file separately.  The manpages, though, I guess you can't do much about.
You could have %files lists like these, but maybe you just prefer to list
everything out:

%files
%defattr(-,root,root,-)
%doc LICENSE SECURITY
%{_sbindir}/corosync*
%config(noreplace) /etc/corosync.conf
%config(noreplace) /etc/ld.so.conf.d/corosync-*.conf
%{_initrddir}/corosync
%{_libexecdir}/lcrso
%dir %{_libdir}/corosync/
%{_libdir}/corosync/*.so.*
%{_mandir}/man8/corosync-objctl.8*
%{_mandir}/man8/corosync_overview.8*
%{_mandir}/man8/logsys_overview.8*
%{_mandir}/man5/corosync.conf.5*

%files devel
%defattr(-,root,root,-)
%doc README.devmap
%{_includedir}/corosync
%{_libdir}/corosync/*.so
%{_mandir}/man3/*
%{_mandir}/man8/cpg_overview.8*
%{_mandir}/man8/evs_overview.8*
%{_mandir}/man8/confdb_overview.8*

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