[Bug 229910] Review Request: Conmux - Console Multiplexor, abstracts how to connect via backend drivers.

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

Summary: Review Request: Conmux - Console Multiplexor, abstracts how to connect via backend drivers.


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





------- Additional Comments From wolfy at nobugconsulting.ro  2007-02-26 14:08 EST -------
Offenders at the first sight:

- I think it would have been better if you have used 
version=0 (or even 0.0..)
release=0.484svn <-- the leading 0 would be used as release tag and increased
for each new version of the spec (for a given snapshot release)
This way the fact that we are dealing with svn snapshots is clearly indicated,
even for those which do not examine the spec or do an actual checkout
- The changelog should contain an entry for the current version; for the moment
there is just one entry which speaks about 0-0.1.20070223svn while the submitted
package is 0-0.1.r484; if 20070223svn and 0.1.r484 are one and the same, please
edit the changelog to fit (do not forget to increase the release tag each time
you submit a new form of the spec file!)
- the license tag should be just "GPL" rather then "GPLv2"
- location of the Conmux.pm in conmux-common does not seem right, the site_perl
directory should be one level higher then the one you install to:
#ll  /usr/lib/perl5/
5.8.5/       5.8.6/       5.8.7/       5.8.8/       site_perl/   vendor_perl/
while your spec creates:
#rpm -qlp conmux-common-0-0.1.r484.fc6.noarch.rpm
/etc/conmux/conmux.conf
/usr/lib/perl5/site_perl/5.8.8/Conmux.pm
- the conmux package includes the whole /etc/conmux directory, overlapping with
conmux-common which includes /etc/conmux/conmux.conf; at install time they would
conflict
- the -client and -common packages are missing the mandatory %defattr line
- since you use service and chkconfig to add/remove the Conmux service, you must
add requirements for them (see the packaging guidelines for details on scriptlets)
- if the /etc/init.d/conmux script would contain restart/reload sections,
rpmlint would be happier (that's a nice-to-have, not a must)
- you create /var/log/conmux; it would be nice if you would also provide a means
for logrotate to handle it. However this is open for debate because you'll need
to %Require logrotate, which OTOH some people might prefer to not use (or
replace with something else).

 As a sidenote, I think that the name "console" used by the client is a bit too
generic and might clash with other console clients


-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.