Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 227811
Summary: | Review Request: hunspell-af - Afrikaans hunspell dictionary | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Caolan McNamara <caolanm> |
Component: | Package Review | Assignee: | manuel wolfshant <wolfy> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | Flags: | wolfy:
fedora-review+
notting: fedora-cvs+ |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2007-02-14 09:26:39 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: | |||
Bug Depends On: | |||
Bug Blocks: | 163779 |
Description
Caolan McNamara
2007-02-08 11:50:29 UTC
Caolan: I am willing to work with you through all dictionaries. Quick review for this package: - unzip is not needed as BR because it's on the exception list (see http://fedoraproject.org/wiki/Extras/FullExceptionList) - adding -q to the %setup line and an empty %build would silence rpmlint (see below) Everything else seems fine - package meets naming guidelines - package meets packaging guidelines - spec file legible, in am. english - source matches upstream , sha1sum fa0dfbe45efd7eeba04e069f2e5987b721ebae71 af_ZA-pack.zip - the package builds in mock, devel/x86_64, generates a noarch (which is consistent with the fact that basically it includes only 3 text files) - the license LGPL stated in the tag is the same as the web site says - there are only 2 files (word lists) + a short doc with instructions and license clearance, so no .la, .pc, static files - no missing BR - MINOR: unneeded BR: unzip - no locales - not relocatable - owns all files/directories that it creates, does not take ownership of other files/dirs - no duplicate files - permissions ok - %clean ok - macro use consistent - rpmlint output: on source: W: hunspell-af setup-not-quiet W: hunspell-af no-%build-section on binary: silent - code, not content - no need for -docs as the only doc is just a 31K text file - nothing in %doc affects runtime - no need for .desktop file The package is APPROVED but before comitting to CVS please fix the three small problems mentioned in the first paragraph. Well, * Timestamps - Keep timestamps. i.e. Use "cp -p" * By the way, some reason you don't want to use %?dist tag? Damn, I missed those :( Mamoru, thank you. * Missing empty %build section. * Added value in the %install loop would be to %lang'ify the files via a file-list and by copying the Locale identifier from the file names, like "%lang(af_ZA) foobar". That depends a bit on the future of %lang in RPM and in related package tools, however. I've seen it in several packages, e.g. kphotoalbum. updated spec and srpm with setup -q, cp -p, %?dist, removed BR unzip and added an empty %build. I'd rather not set %lang on the files as they're dictionaries rather than documentation or translations, I'm not comfortable that it fits perfectly conceptually to mark dicts that way as well. - Source0 is not an absolute URL. oky doky, I don't see a section in our guidelines about this, but not a problem. Absolute URL for Source: in the current version (In reply to comment #7) > oky doky, I don't see a section in our guidelines about this, but not a problem. > Absolute URL for Source: in the current version Where have you been throughout the years Fedora exists? This rule had even been enforced during Fedora.us. But I realize, the Guidelines are a bit vague on this. The rationale behind this: Without an absolute URL, it's unnecessary hard to * verify if the source tarball matches upstream or if it has been compromised. * to track upstream activities (Is the tarball still there, has a new version been released etc.) You will also want to have a look at /usr/bin/spectool (from package rpmdevtools, available from FE). The wiki says "- MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. Reviewers should use md5sum for this task." Maybe we could rephrase that in a form which makes clear that full URL is mandatory (and sha1sum is also allowed as alternative) ? Since this version is, just as you said, a bit vague, I was never picky about full URL, as long as the URL + %name + %version led to a valid result. Caolan, it looks like you have fixed all recommendations that were suggested and that no one has anything else to comment. I suggest considering this package as approved and using this spec as template for the others. I will be glad to review them all and I hope that other eyes will keep verifying, just like before. I also hope I won't miss anything :) PS: in the future, please increase the release tag and add an entry in the %Changelog when you modify the spec. 27535 (hunspell-af): Build on target fedora-development-extras succeeded. |