Bug 470508 - Review Request: Ajaxterm - A web-based terminal
Review Request: Ajaxterm - A web-based terminal
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Susi Lehtola
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-11-07 08:47 EST by Ruben Kerkhof
Modified: 2008-11-22 08:18 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-11-22 08:18:58 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
susi.lehtola: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Ruben Kerkhof 2008-11-07 08:47:31 EST
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 08:36:10 EST
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 08:49:57 EST
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 10:07:44 EST
And still, require the packages chkconfig and initscripts instead of /sbin/chkconfig and /sbin/service.
Comment 4 Ruben Kerkhof 2008-11-09 14:20:47 EST
Why, I don't remember reading that in the packaging guidelines?
Comment 5 Susi Lehtola 2008-11-09 14:27:27 EST
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 14:46:37 EST
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 15:07:44 EST
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 16:20:06 EST
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 07:30:26 EST
(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 17:02:36 EST
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-18 20:45:56 EST
I assume you wanted a F-10 branch here as well? 

cvs done with F-10 added.
Comment 12 Ruben Kerkhof 2008-11-22 08:18:58 EST
Thanks for the review Jussi.

Package build and imported into rawhide.

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