[Bug 225286] Merge Review: aspell

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

Summary: Merge Review: aspell


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





------- Additional Comments From jspaleta@xxxxxxxxx  2007-02-03 16:46 EST -------
Okay doing the merge review.

Summary:
There are things which need to be changed to meet the established review
guidelines.  Which I will go over in bloody detail below. 
However let me first say that I certaintly in agreement with Enrico concerning
the perl dep issue and I would very much recommend that the perl script be moved
to a subpackage or pushed into the docs section..whatever it takes to keep perl
from being a hard requirement of the base package.  Though the perl issue is
outside of my strict mandate to for the merge review.

I have attached a spec file diff which incorporate an attempt to fix the issues
I list as blockers below.  The package maintainer needs to review each of the
spec changes and make sure they are valid, the work, and aren't contentious.
If the package owner has a problem with anything I suggest, they need to report
back into this bug so we can talk about it.

Okay so on with the review

aspell
Checklist:  + GOOD  - BAD
+ rpmlint... see the notes at the end. I've rolled in changes into the spec from
the rpmlint log
+ packagename is fine
+ specfile name is fine
+ license check 
	LGPL in spec tag matches COPYING file in upstream source and COPYING file
included in doc section
+ spec is english-ish
+ md5sum check of sources
17fd8acac6293336bcef44391b71e337  aspell-0.60.5.tar.gz from SOURCE URL
17fd8acac6293336bcef44391b71e337  rpmbuild/SOURCES/aspell-0.60.5.tar.gz from SRPM
+ mock build  as done by matt

+ no buildrequires look good.

+ shared libs look fine
 ldconfig is being called in post preun as expected 
+ not designed to be relocatable
+ no duplicates in the files section
+ file permissions look okay to me
-  locales.. not so good	
+ headers in devel subpackage no static libs
+ docs section looks fine
+ no gui apps
+ no obvious duplicate file/directory ownership

BAD:
MUSTFIX: The spec file MUST handle locales properly. This is done by using the
%find_lang macro. Using %{_datadir}/locale/* is strictly forbidden.
MUSTFIX: Need to rm -rf $RPM_BUILD_ROOT  in the install section.
MUSTFIX: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'
(for directory ownership and usability).
MUSTFIX: remove Prereq and use Requires(x) syntax for scriptlets

rpmlint run from matt/dell:
... notes from reviewer inline

rpmlint on ./aspell-debuginfo-0.60.5-2.fc7.i386.rpm
rpmlint on ./aspell-devel-0.60.5-2.fc7.i386.rpm
W: aspell-devel summary-ended-with-dot Static libraries and header files for
Aspell development.
... fixed in spec diff
rpmlint on ./aspell-0.60.5-2.fc7.i386.rpm
E: aspell obsolete-not-provided ispell
... looks like a valid obsolete

E: aspell obsolete-not-provided aspell-de
E: aspell obsolete-not-provided aspell-fr
E: aspell obsolete-not-provided aspell-ca
E: aspell obsolete-not-provided aspell-da
E: aspell obsolete-not-provided aspell-es
E: aspell obsolete-not-provided aspell-it
E: aspell obsolete-not-provided aspell-nl
E: aspell obsolete-not-provided aspell-no
E: aspell obsolete-not-provided aspell-sv
... since these are versioned obsoletes which have newer versions
in the package tree these are completely bogus error messages.
In fact, does aspell need to keep these obsoletes at all?

E: aspell obsolete-not-provided aspell-pt_BR
... this doesnt have a current package which provides this. 
This is most likely a valid obsoletes, and a dead-end.. so no error.

E: aspell obsolete-not-provided aspell-config
... this is no longer provided by anything.  Is this a valid obsoletes?
Is the pspell-config binary equivalent? If so can an aspell-config symlink
be added and a Provides put in place? I'm honestly not sure about this error.
Package owner will have to provide some backstory concerning this obsoletes.  

W: aspell file-not-utf8 /usr/share/info/aspell.info.gz
... ewww uhm this is an upstream issue. I dont think we can just run iconv
on a gzipped info file... perhaps rpmlint is just being silly here.

rpmlint on ./aspell-0.60.5-2.fc7.src.rpm
W: aspell prereq-use /sbin/install-info
... fixed with Requires(post) and Requires(preun) in spec diff

W: aspell unversioned-explicit-provides pspell
W: aspell unversioned-explicit-obsoletes ispell
W: aspell unversioned-explicit-obsoletes pspell
W: aspell unversioned-explicit-obsoletes aspell-pt_BR
W: aspell unversioned-explicit-obsoletes aspell-config
W: aspell unversioned-explicit-provides pspell-devel
W: aspell unversioned-explicit-obsoletes pspell-devel
W: aspell macro-in-%changelog doc
... reworded the changelog entry spec diff
W: aspell macro-in-%changelog serial
... reworded the changelog entry in spec diff

-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.

_______________________________________________
Fedora-package-review mailing list
Fedora-package-review@xxxxxxxxxx
http://www.redhat.com/mailman/listinfo/fedora-package-review

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