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@xxxxxxxxxxxxxxxxxx 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review