[Bug 226321] Merge Review: psgml

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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=226321

--- Comment #7 from Alex Lancaster <alexl@xxxxxxxxxxxxxxxxxxxxx> 2010-02-08 17:48:00 EST ---
(In reply to comment #2)

New build here:

http://koji.fedoraproject.org/koji/buildinfo?buildID=155021

Diff between new spec and old spec in CVS:

http://cvs.fedoraproject.org/viewvc/rpms/psgml/devel/psgml.spec?r1=1.24&r2=1.25

> Summary: No dot at the end please

Fixed.

> name: Capital letter at the beginning

Fixed.

> Prereq: should not be used, I doubt it really is one. sgml-common is a Requires and BuildRequires.

Fixed.

> Is the license tag really correct? I could not find anything in the sources.

> URL tag is missing

Added.

> BuildRoot: is invalid, should be 
>   %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
> see https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag

Fixed.

> Prefix: /usr: drop this completely. Will be set automatically and using
> absolute paths is strictly forbidden

Dropped.

> Requires: emacs is missing

Added.

> %define psgmldir %{prefix}/share/emacs/site-lisp/psgml/: use %global, see
> https://fedoraproject.org/wiki/PackagingDrafts/global_preferred_over_define
> %{prefix}/share/ should be %{_datadir} here

Fixed.

> %description: line breaks at 80 characters 

Line breaking was at 70 chars, now at 80 chars.


> %build:

> PATH=/usr/bin:$PATH this is most likely unnecessary

Removed.

> ./configure --prefix=/usr: Hardcoded paths are strictly forbidden, just use
> %configure here.

Done.

> Use parallel make if possible. If not, write a comment, see
> https://fedoraproject.org/wiki/Packaging/Guidelines#Parallel_make
> 
> During build I see a lot of errors like this one:
>   Warning: !! The file uses old-style backquotes !!
>   This functionality has been obsolete for more than 10 years already
>   and will be removed soon.  See (elisp)Backquote in the manual.
> Please look into this.

psgml is a very old and currently unmaintained upstream AFAIK.  I will look
into whether it's possible to easily fix this in a later revision.

> %install:
> 
> When creating psgml-init.el you should also use macros, %{psgmldir} instead of 
> /usr/share/emacs/site-lisp/psgml, %{_sysconfdir} instead of /etc and so on
> 
> mkdir -p $RPM_BUILD_ROOT/%{prefix}/share/sgml/cdtd: use %{_datadir} instead of
> %{prefix}/share

Used %{_datadir} and %{_sysconfdir} throughout .spec

> Timestamps are not preserved during install, add INSTALL="install -p"

> The install-info scriptlets lack the "|| :" at the end of the line and the
> %postun scriptlet is wrong, should be run in %preun, see
> https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#Texinfo

Added "|| :" and made postun script a preun script as per above link.

> %files:

> %defattr should be (-,root,root,-)

Fixed

> %{prefix}/share/sgml/cdtd should be %{_datadir}/sgml/cdtd

Fixed.

> ChangeLog and psgml.texi are missing from %doc

Added.

> ChangeLog, psgml.texi and psgml.info.gz are not UTF-8, please convert them as
> described in
> https://fedoraproject.org/wiki/PackageMaintainers/PackagingTricks#Convert_encoding_to_UTF-8

Fixed.


> Now the full checklist. All FIX or ??? items are explained above.
> 
> FIX - MUST: rpmlint
> $ rpmlint noarch/psgml-1.2.5-9.fc12.noarch.rpm 
> psgml.noarch: W: summary-ended-with-dot A GNU Emacs major mode for editing SGML
> documents.
> psgml.noarch: E: tag-not-utf8 %changelog
> psgml.noarch: W: no-url-tag
> psgml.noarch: W: file-not-utf8 /usr/share/info/psgml.info.gz
> 1 packages and 0 specfiles checked; 1 errors, 3 warnings.

New rpmlint run:

rpmlint noarch/psgml-1.2.5-12.fc13.noarch.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.


> FIX - MUST: The package meets the Packaging Guidelines.

Should be fixed now.

> ??? - MUST: The License field in the package spec file matches the actual
> license.

Headers in .el files contain "GPLv2+" license, e.g.:

;; This program is free software; you can redistribute it and/or
;; modify it under the terms of the GNU General Public License
;; as published by the Free Software Foundation; either version 2
;; of the License, or (at your option) any later version.

which is GPLv2+ AFAIK.

> FIX - MUST: The license file from the source package is included in %doc.

Upstream didn't include separate COPYING file, not strictly necessary and since
upstream is dead, not much point in filing a bug there.
.
> FIX - MUST: The package consistently uses macros, as described in the macros
> section of Packaging Guidelines.

I think this should be fixed now.

> SHOULD Items:
> ??? - SHOULD: If the source package does not include license text(s) as a
> separate file from upstream, the packager SHOULD query upstream to include it.

See above.

> FIX - SHOULD: If scriptlets are used, those scriptlets must be sane. This is
> vague, and left up to the reviewers judgement to determine sanity.

Should be fixed now.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]