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@xxxxxxxxx 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review