Bug 470508

Summary: Review Request: Ajaxterm - A web-based terminal
Product: [Fedora] Fedora Reporter: Ruben Kerkhof <ruben>
Component: Package ReviewAssignee: Susi Lehtola <susi.lehtola>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, notting, susi.lehtola
Target Milestone: ---Flags: susi.lehtola: fedora-review+
kevin: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2008-11-22 13:18:58 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Ruben Kerkhof 2008-11-07 13:47:31 UTC
Spec URL: http://ruben.fedorapeople.org/Ajaxterm.spec
SRPM URL: http://ruben.fedorapeople.org/Ajaxterm-0.10-1.fc9.src.rpm
Description:
Ajaxterm is a web based terminal. It was totally inspired and works almost 
exactly like http://anyterm.org/ except it's much easier to install.

Comment 1 Susi Lehtola 2008-11-08 13:36:10 UTC
Read http://fedoraproject.org/wiki/Packaging/UsersAndGroups and make necessary corrections to spec file (Requires and adding users).

Comment 2 Ruben Kerkhof 2008-11-09 13:49:57 UTC
Thanks, corrected.


New version here:
Spec URL: http://ruben.fedorapeople.org/Ajaxterm.spec
SRPM URL: http://ruben.fedorapeople.org/Ajaxterm-0.10-2.fc9.src.rpm

Comment 3 Susi Lehtola 2008-11-09 15:07:44 UTC
And still, require the packages chkconfig and initscripts instead of /sbin/chkconfig and /sbin/service.

Comment 4 Ruben Kerkhof 2008-11-09 19:20:47 UTC
Why, I don't remember reading that in the packaging guidelines?

Comment 5 Susi Lehtola 2008-11-09 19:27:27 UTC
At least that's the way it's in

http://fedoraproject.org/wiki/Packaging/SysVInitScript

Besides, requiring a package is AFAIK faster than going through the file lists, although the /sbin list is downloaded automatically anyways.

Comment 6 Ruben Kerkhof 2008-11-09 19:46:37 UTC
Ah, you're right.

New version build:
Spec URL: http://ruben.fedorapeople.org/Ajaxterm.spec
SRPM URL: http://ruben.fedorapeople.org/Ajaxterm-0.10-3.fc9.src.rpm

Comment 7 Susi Lehtola 2008-11-09 20:07:44 UTC
Well, I might as well review the package.

- MUST: The package must own all the directories it creates.

%{_datadir}/ajaxterm/*

should be

%{_datadir}/ajaxterm

- MUST: License is incorrect - sarissa* files are licensed under LGPLv2+

License: Public Domain and LGPLv2+



Other than that I cannot do now, since the AjaxTerm site is down and I cannot check the sources.

Comment 8 Ruben Kerkhof 2008-11-09 21:20:06 UTC
Thanks, fixed both points.

New version:
Spec URL: http://ruben.fedorapeople.org/Ajaxterm.spec
SRPM URL: http://ruben.fedorapeople.org/Ajaxterm-0.10-4.fc9.src.rpm

Comment 9 Susi Lehtola 2008-11-17 12:30:26 UTC
(In reply to comment #8)
> Thanks, fixed both points.
> 
> New version:
> Spec URL: http://ruben.fedorapeople.org/Ajaxterm.spec
> SRPM URL: http://ruben.fedorapeople.org/Ajaxterm-0.10-4.fc9.src.rpm

Where do you set LANG=C (or unset LANG)?  Ajaxterm only supports latin1.

Spec cleanup: Change
%dir %{_datadir}/ajaxterm
%{_datadir}/ajaxterm/*

to simply
%{_datadir}/ajaxterm


Source and license matches upstream.

****************************
The package is APPROVED.
****************************

Do the fixes mentioned above before importing to CVS.

Comment 10 Ruben Kerkhof 2008-11-18 22:02:36 UTC
Thanks, I'll fix the above mentioned points before importing.

New Package CVS Request
=======================
Package Name: Ajaxterm
Short Description: A web-based terminal
Owners: ruben
Branches: F-8 F-9 EL-4 EL-5

Comment 11 Kevin Fenzi 2008-11-19 01:45:56 UTC
I assume you wanted a F-10 branch here as well? 

cvs done with F-10 added.

Comment 12 Ruben Kerkhof 2008-11-22 13:18:58 UTC
Thanks for the review Jussi.

Package build and imported into rawhide.