[Bug 491694] Review Request: Anyterm - Web based terminal emulator

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


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





--- Comment #23 from Mohammed Morsi <mmorsi at redhat.com>  2009-07-13 18:18:57 EDT ---
(In reply to comment #22)
> Still not a formal review.
> 
>  == FAIL ==
> 
>  * MUST: The package must meet the Packaging Guidelines .
> 
> See below.
> 
>  * MUST: The package must be licensed with a Fedora approved license and meet
> the Licensing Guidelines .

Is this a separate issue or does this relate to the next one? The current spec
file includes "License: GPLv2+" which should satisfy this requirement.


>  * MUST: The License field in the package spec file must match the actual
> license. [3]
> 
> No license is provided for
> http://svn.anyterm.org/anyterm/trunk/src/sun_forkpty.h

Since I'm building from the latest release, 1.1.29, and that file doesn't exist
in there (only in trunk) this is not an issue: 

  http://svn.anyterm.org/anyterm/tags/releases/1.1/1.1.29/src/

(just out of curiosity if it was an issue, what would I need to do, eg would
this something we couldn't proceed with until it was resolved upstream? once
again, not an issue but I am interested)

> 
>  * MUST: If (and only if) the source package includes the text of the
> license(s) in its own file, then that file, containing the text of the
> license(s) for the package must be included in %doc.[4]
> 
> %doc must be in the %files section.

Done, moved %doc to %files


> 
>  * MUST: A package must own all directories that it creates. If it does not
> create a directory that it uses, then it should require a package which does
> create that directory. [12]
> 
> Needs to req. httpd.

I'm wondering if we could make this optional. Yes we do install anyterm.conf to
%{_sysconfdir}/httpd/conf.d/ but nothing in anyterm explicitly depends on httpd
and it can be run 100% fine as is without it. I think it would be really
worthwhile to make this optional so as not to impose the httpd requirement on
sysadmins who want to run anyterm standalone (for example in a scenario where
httpd / anytermd are running on seperate machines). Is there anyways to do this
(what are your thoughts about a seperate anyterm-httpd package?)


> 
>  * MUST: Each package must consistently use macros. [16]
> 
> Mixed use of %{buildroot} and $RPM_BUILD_ROOT and mixed %{__foo} vs. foo.
> 
substituted $RPM_BUILD_ROOT w/ %{buildroot}, but not sure which 'foo' you are
referring too, I tried to use all the predefined %{} macros that I could where
appropriate


> 
> 
>  == Would be nice to fix ==
> 
>  * SHOULD: The description and summary sections in the package spec file should
> contain translations for supported Non-English languages, if available. [28]

I'm not sure how to approach doing these translations / which are needed


>  * SHOULD: If scriptlets are used, those scriptlets must be sane. This is
> vague, and left up to the reviewers judgement to determine sanity. [31]
> 
> Shouldn't it use >/dev/null instead of &>/dev/null for useradd/groupadd?
> They're basically sane, though.

Which way is standard? Googling for this, I find most specs redirect both
stdout / stderr
(or even just stderr)

http://www.google.com/#hl=en&q=groupadd+%2Fdev%2Fnull+filetype%3Aspec+&aq=f&oq=groupadd+filetype%3Aspec&aqi=&aq=&oq=&aqi=&aq=f&oq=&aqi=&fp=Xmf0jJ9P_V0

> 
>  * Additionally:
> 
> anyterm-cmd should perhaps be moved to %{_libexecdir}/%{name}/anyterm-cmd

Done, moved it from bindir to libexecdir

New SPEC and SRPM (bumped release):
http://mohammed.morsi.org/blog/files/anyterm.spec
http://mohammed.morsi.org/blog/files/anyterm-1.1.29-6.fc10.src_.rpm

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.