[Bug 236642] Review Request: Revisor - Revisor GUI

[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: Revisor - Revisor GUI


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





------- Additional Comments From jspaleta at gmail.com  2007-05-04 03:50 EST -------
The GOOD
+ naming is good
+ specfile name matches base package name 
+ specfile written in english-ese and is legible
+ included source md5sum checks with upstream source as listed in SOURCE0 url
9b1bb4207f9c8a64609d1007420955ef  revisor-2.0.1.tar.gz
+ builds on x86 fedora-development in mock
+ no locales
+ not relocatable
+ clean section is okay
+ consistent use of macros
+ permissible code and content
+ items in doc are not runtime necessary
+ does not obviously own files from another package 
+ directory ownership of parent directories is accounted for in package deps.
+ No .la files
+ No devel subpackage
+ Need need to for shared libraries sciptlets
+ no need for scriptlets.

The BAD
- Licensed as GPL but COPYING file NOT included in docs!!

- install section has a problem
needs to include a desktop-file-install stanza to install the desktop file
correctly, and require desktop-file-utils Refer to
http://fedoraproject.org/wiki/Packaging/Guidelines#desktop

? permissions on pilgrim.py
Is pilgrim.py meant to be run stand-alone as an executable. You should either
strip the 
shell invocation at the top of the file or make it executable.


The Suggestions:
You can remove pam from the requires list, usermode requires pam. I know I know
i suggested it originally, based on just the directory ownership crap. But
taking a closer look the pam requires is redundant.

I haven't actually used this yet. Is there a simple example walkthrough on
usage? Like how to make a stupidly simple livedvd image or something, so I dont
have to think about the package selection but I can test the wizard interface.


rpmlint revisor-2.0.1-4.fc7.noarch.rpm
E: revisor non-executable-script
/usr/lib/python2.5/site-packages/revisor/pilgrim.py 0644
W: revisor non-conffile-in-etc /etc/revisor/conf.d/pungi-fc6-i386.conf
W: revisor non-conffile-in-etc /etc/revisor/conf.d/revisor-fc6-ppc.conf
W: revisor non-conffile-in-etc /etc/revisor/conf.d/pungi-f7-i386.conf
W: revisor non-conffile-in-etc /etc/revisor/conf.d/pungi-fc6-ppc.conf
W: revisor non-conffile-in-etc /etc/revisor/conf.d/pungi-fc6-x86_64.conf
W: revisor non-conffile-in-etc /etc/revisor/conf.d/revisor-fc6-x64_86.conf
W: revisor non-conffile-in-etc /etc/revisor/conf.d/revisor-fc6-i386.conf
W: revisor non-conffile-in-etc /etc/revisor/conf.d/sample-ks.cfg
W: revisor non-conffile-in-etc /etc/revisor/revisor.conf
W: revisor non-conffile-in-etc /etc/security/console.apps/revisor
W: revisor non-conffile-in-etc /etc/revisor/conf.d/revisor-f7-i386.conf
W: revisor non-conffile-in-etc /etc/pam.d/revisor

-- warnings are bogus, might want to patch the pilgrim.py to either be
executable or to strip
the intepreter from the first line.

rpmlint revisor-2.0.1-4.fc7.src.rpm
W: revisor strange-permission revisor.spec 0600
-- not important, but you might consider making it world and group readable by
default.

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