[Bug 226800] Review Request: emacs-bbdb - email database for Emacs

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: emacs-bbdb - email database for Emacs


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





------- Additional Comments From jonathan.underwood at gmail.com  2007-02-20 18:35 EST -------
Hi Tom,

Interestingly I submitted this around the same time as you, so please look at
the spec file in bug 227230, as that contains a number of things I think you
should consider, including:

0) You're building and packaging *only* the bbdb bindings for VM, whereas bbdb
comes with bindings for Gnus and MH-E, both included with Emacs - you need to do
make all to build those as well.

1) BBDB requires TeX for the bbdb-print functionality to work, as bbdb-print
creates a TeX file, and so I included a Requires: tetex. I'm on the fence with
this one, I have to say.

2) Your package description doesn't mention that the BBDB is an add on for Emacs
- it should.

3) You should use $RPM_BUILD_ROOT *or* %{buildroot} in your spec consistently,
not a mixture of both.

4) You need to remove [ "$RPM_BUILD_ROOT" != "/" ] && from the %clean section

5) There are a number of shell scripts and extra bits which are very useful to
bbdb users - I packaged these as documentation, along with the ChangeLog with 
    %doc bits misc utils ChangeLog
in the %files section - I strongly urge you to do the same.

6) Assuming you do (5) above, you'll need to make the *.pl files non executeable
with a 
    find -name '*.pl' -exec chmod -x {} \;
in the %install section, and you'll also need to remove some irrelevant files:
    rm -f bits/make.bat
    rm -f utils/Makefile*

7) What was your reason for putting bbdb-autoloads in site-start.d ? If I recall
correctly, this is unecessary, all of the files can live happily in
site-lisp/bbdb (which is on the load-path by default). Perhaps I missed
something here though.

8) The make install-pkg target in the bbdb makefile is totally useless for our
purposes - for readability I'd really recommend not using that, but rather doing
a few simple copies into the buildroot. Example from my spec:
# There is no usable install make rule - install by hand.
%define lispdir %{_datadir}/emacs/site-lisp/bbdb
mkdir -p $RPM_BUILD_ROOT%{lispdir}
cp lisp/*.{elc,el} $RPM_BUILD_ROOT%{lispdir}

%define texdir %{_datadir}/texmf/tex/generic/bbdb
mkdir -p $RPM_BUILD_ROOT%{texdir}
cp tex/*.tex $RPM_BUILD_ROOT%{texdir}

mkdir -p $RPM_BUILD_ROOT%{_infodir}
cp texinfo/bbdb.info $RPM_BUILD_ROOT%{_infodir}

9) I think your BuildRequires for emacs-vm-el is unecessary, as although
warnings will pop up, I don't think they matter. I *may* be wrong here though. I
sort of hope so, since I recently added a patch to the emacs-vm package (the
vmrf patch) which ultimately will lead to a BuildRequires: emacs-bbdb, at which
point we'll have a circular dependency. 

I need to think more about point 9.

Also, if you want a co-maintainer, I'm happy to do that.

Great to see someone else interested in bbdb!

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