Bug 222888 - Review Request: Tiquit - A PHP5-compatible help desk incident tracking/knowledgebase system
Review Request: Tiquit - A PHP5-compatible help desk incident tracking/knowle...
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jason Tibbitts
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-01-16 14:03 EST by Jon Ciesla
Modified: 2007-11-30 17:11 EST (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-05-10 07:33:25 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
tibbs: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Jon Ciesla 2007-01-16 14:03:46 EST
Spec URL: http://zanoni.jcomserv.net/extras/tiquit/tiquit.spec
SRPM URL: http://zanoni.jcomserv.net/extras/tiquit/tiquit-2.4-1.src.rpm
Description: Tiquit offers email notification, parent/child tickets, and
comprehensive search and reporting functionality.
Comment 1 Jason Tibbitts 2007-05-06 02:23:14 EDT
I'll take a look.
Comment 2 Jason Tibbitts 2007-05-06 03:11:24 EDT
Builds fine and rpmlint is silent.

Source0 should be a URL; I used the usual
  http://dl.sf.net/%{name}/%{name}-%{version}.tar.gz
so that I could grab the upstream source for comparison.

The link to config.php looks a bit odd:
  /usr/share/tiquit/config.php -> ../../..//etc/tiquit/config.php
%{_sysconfdir} includes a leading slash.  Normally I'd ask if there's a nicer
way to do this than via a symlink, but it's pretty obvious from the code that
you'd have to patch every source file.

Since this comes up locked down to localhost, it's probably worth adding a
README.Fedora file containing information about why you've done that, what the
user needs to in order to enable things, and if there's any mandatory
configuration they need to do before turning it on.

I'm not sure your dependency on php-gd does what you want it to.  Note that it
won't pull in a PHP interpreter, just php-common.

I'm not sure why you have
  %dir %{tiquitdir}
  %{tiquitdir}/*
instead of just
  %{tiquitdir}

* source files match upstream:
   1c7aefbde49ee7ee4f946d60218a9f19f5a5236df67f3e401ffa76f2ecfc563c  
   tiquit-2.4.tar.gz
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text included in package.
* latest version is being packaged.
* BuildRequires are proper (none needed)
* %clean is present.
* package builds in mock (development, x86_64).
* package installs properly
* debuginfo package looks complete.
* rpmlint is silent.
? final provides and requires are sane:
   config(tiquit) = 2.4-1.fc7
   tiquit = 2.4-1.fc7
  =
   config(tiquit) = 2.4-1.fc7
   php-gd

* %check is not present; no test suite upstream.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no scriptlets present.
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
Comment 3 Jon Ciesla 2007-05-07 09:25:10 EDT
Fixed Source0.

The symlink to the config is the result of the review for limph, which I also
wrote, which uses the same sort or structure.  This was suggested by the reviewer.

Added tiquit-README.fedora. Is it OK if this is in /usr/share/doc, and not
/usr/share/doc/%{name}-%{version}?  %doc won't take %SOURCE macros and this is
the best solution I could find.

Fixed the PHP Requires.

Spec URL: http://zanoni.jcomserv.net/extras/tiquit/tiquit.spec
SRPM URL: http://zanoni.jcomserv.net/extras/tiquit/tiquit-2.4-2.src.rpm

Comment 4 Ralf Corsepius 2007-05-07 09:43:52 EDT
(In reply to comment #3)

> Added tiquit-README.fedora. Is it OK if this is in /usr/share/doc, and not
> /usr/share/doc/%{name}-%{version}?
No.

>  %doc won't take %SOURCE macros and this is
> the best solution I could find.
If you copy it into the the builddir, %doc will work.
Normally, a 
cp %{SOURCE<X>} .
somewhere in %prep will do.
Comment 6 Jason Tibbitts 2007-05-07 14:17:04 EDT
The symlink itself is fine; I was only commenting on the doubled slash in the
target which is caused because %{_sysconfdir} includes a leading slash and you
include one yourself, so two end up in the symlink.  It's not a big deal; I
guess it would be a problem if we ever got some sort of double-slash magic that
some systems have.  Fix it if you like.

Anyway, the only real issues (no PHP interpreter in the dependency chain and
lack of Fedora-specific setup documentation) has been fixed, so I'd say
everything's good to go.

APPROVED
Comment 7 Jon Ciesla 2007-05-07 14:30:30 EDT
New Package CVS Request
=======================
Package Name: tiquit
Short Description: A PHP5-compatible help desk incident tracking/knowledgebase
system
Owners: limb@jcomserv.net
Branches: FC-5 FC-6
InitialCC: 
Comment 8 Jon Ciesla 2007-05-10 07:33:25 EDT
Imported and built.  Thanks for the review!
Comment 9 Jon Ciesla 2007-07-12 11:22:07 EDT
Package Change Request
======================
Package Name: tiquit
New Branches: EL-4 EL-5
Comment 10 Kevin Fenzi 2007-07-12 13:07:52 EDT
cvs done. 
Comment 11 Jon Ciesla 2007-08-16 13:25:50 EDT
Package Change Request
======================
Package Name: tiquit
New Branches: F-7

Not sure why this isn't there. . .
Comment 12 Kevin Fenzi 2007-08-16 22:02:30 EDT
cvs done. It's not there because you didn't request it before? ;) 

Note You need to log in before you can comment on or make changes to this bug.