Bug 184000 - Review Request: emacs-vm
Review Request: emacs-vm
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Kevin Fenzi
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-03-04 16:44 EST by Jonathan Underwood
Modified: 2007-11-30 17:11 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-06-22 15:08:54 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Jonathan Underwood 2006-03-04 16:44:33 EST
Spec Name or Url: http://physics.open.ac.uk/~ju83/emacs-vm.spec
SRPM Name or Url: http://physics.open.ac.uk/~ju83/emacs-vm-7.19-1.src.rpm
Description: 
VM (View Mail) is an Emacs subsystem that allows UNIX mail to be read and disposed of within Emacs.  Commands exist to do the normal things
expected of a mail user agent, such as generating replies, saving messages to folders, deleting messages and so on.  There are other more advanced commands that do tasks like bursting and creating digests, message forwarding, and organizing message presentation according to various criteria.

****I require a sponsor**** (I have also submitted another package for review: emacs-muse)
Comment 1 Mike McGrath 2006-03-05 10:39:57 EST
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.
Comment 2 Jens Petersen 2006-04-18 05:33:55 EDT
Valid points.
Comment 3 Kevin Fenzi 2006-05-18 21:27:17 EDT
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). 
Comment 4 Jonathan Underwood 2006-05-22 20:15:46 EDT
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...).
Comment 5 Kevin Fenzi 2006-06-20 02:10:03 EDT
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.
Comment 6 Jonathan Underwood 2006-06-20 06:45:17 EDT
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.
Comment 7 Jens Petersen 2006-06-20 07:27:35 EDT
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.:)

Comment 8 Jonathan Underwood 2006-06-22 15:08:54 EDT
Package imported.

Group set as Applications/Internet.
Added a Requires:%{version}-%{release} to the subpackage.

Closing NEXTRELEASE.

Many thanks again.

Note You need to log in before you can comment on or make changes to this bug.