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 184000
Summary: | Review Request: emacs-vm | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Jonathan Underwood <jonathan.underwood> |
Component: | Package Review | Assignee: | Kevin Fenzi <kevin> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | coldwell, imlinux, petersen |
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: | 2006-06-22 19:08:54 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
Jonathan Underwood
2006-03-04 21:44:33 UTC
I cannot sponsor you so just a few comments: The version in %changelog should be listed as: 7.19-1 You may want to create the init file as a separate source. Valid points. If you could update the package to reflect the new 'emacs-common-$name' I can see about doing a review. (removing FE-NEEDSPONSOR as you are now sponsored). Hi Kevin, The emacs-common-$name scheme applies only when a package is meant for multiple emacs flavours - the emacs-vm package here targets only GNU Emacs, since VM is the mailer shipped with XEmacs, and so a package is not required. Cleanups to spec file and updated SRPM: http://physics.open.ac.uk/~ju83/emacs-vm.spec http://physics.open.ac.uk/~ju83/emacs-vm-7.19-2.src.rpm It's a pretty straightforward package, so I don't think there's too much out of whack with it (famous last words...). Sorry, I meant to review this quite a while back... OK - Package name OK - Spec file matches base package name. OK - Meets Packaging Guidelines. OK - License (GPL) OK - License field in spec matches OK - License file included in package OK - Spec in American English OK - Spec is legible. OK - Sources match upstream md5sum: 7866f6243e398d76ae32356a4af76fa3 vm-7.19.tar.gz 7866f6243e398d76ae32356a4af76fa3 vm-7.19.tar.gz.1 OK - Package compiles and builds on at least one arch. n/a - Package needs ExcludeArch OK - BuildRequires correct n/a - Spec handles locales/find_lang n/a - Spec has needed ldconfig in post and postun n/a - Package is relocatable and has a reason to be. OK - Package owns all the directories it creates. OK - Package has no duplicate files in %files. OK - Package has %defattr and permissions on files is good. OK - Package has a correct %clean section. OK - Spec has consistant macro usage. OK - Package is code or permissible content. n/a - -doc subpackage needed/used. OK - Packages %doc files don't affect runtime. n/a - Headers/static libs in -devel subpackage. n/a - .pc files in -devel subpackage. n/a - .so files in -devel subpackage. n/a - -devel package Requires: %{name} = %{version}-%{release} n/a - .la files are removed. n/a - Package is a GUI app and has a .desktop file OK - Package doesn't own any directories other packages own. See below - No rpmlint output. Issues: 1. Should the Requires for the el subpackage be: 'Requires: %{name} = %{version}-%{release}' instead of just 'Requires: %{name} = %{version}' That could cause some confusion down the road. 2. You have the Group as 'Applications/Editors' Since this is a mail reader perhaps one of: Applications/Communications Applications/Internet Applications/Productivity would be more approprate? 3. One (ignoreable) rpmlint warning: W: emacs-vm-el no-documentation As none of those are blockers, this package is APPROVED. You may want to look at items 1 and 2 as you are importing. Hi Kevin, Thanks for taking the time to review, much appreciated. I think you're probably right - Applications/Communications may be the appropriate group. I'll ponder on that. And regarding point one - yes version-release would be better. Will make those changes. mew and wl have been Applications/Internet FWIW. (My initial reaction on seeing this discussion was that "Application/*" should be for desktop applications.... but doesn't seem to have been the case always.:) Package imported. Group set as Applications/Internet. Added a Requires:%{version}-%{release} to the subpackage. Closing NEXTRELEASE. Many thanks again. |